po 12. 1. 2026 v 14:22 odesÃlatel Costa Shulyupin <[email protected]> napsal: > > The patch "Replace atoi() with a robust strtoi()" introduced a bug > in parse_cpu_set(), which relies on partial parsing of the input > string. > > Restore the original use of atoi() in parse_cpu_set(). > > Add a unit test to prevent accidental regressions. > > Fixes: 7e9dfccf8f11 ("rtla: Replace atoi() with a robust strtoi()") > > Signed-off-by: Costa Shulyupin <[email protected]>
The convention is not to insert a blank line between Fixes and Signed-off-by. > --- > tools/tracing/rtla/Makefile | 3 ++ > tools/tracing/rtla/src/utils.c | 10 ++-- > tools/tracing/rtla/tests/Makefile | 12 +++++ > tools/tracing/rtla/tests/test_utils.c | 74 +++++++++++++++++++++++++++ > 4 files changed, 93 insertions(+), 6 deletions(-) > create mode 100644 tools/tracing/rtla/tests/Makefile > create mode 100644 tools/tracing/rtla/tests/test_utils.c > Could you split this into three commits? 1. Fix the bug. 2. Add unit test framework. 3. Add the specific unit tests for utils. IMHO, it would be easier to modify the test in tests/timerlat.t to cover this case for now, and implement unit tests later in an entirely separate patchset. (See comments for the unit tests below.) > diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile > index 2701256abaf3..1805916c7dba 100644 > --- a/tools/tracing/rtla/Makefile > +++ b/tools/tracing/rtla/Makefile > @@ -109,7 +109,10 @@ clean: doc_clean fixdep-clean > $(Q)rm -f rtla rtla-static fixdep FEATURE-DUMP rtla-* > $(Q)rm -rf feature > $(Q)rm -f src/timerlat.bpf.o src/timerlat.skel.h > example/timerlat_bpf_action.o > + > check: $(RTLA) tests/bpf/bpf_action_map.o > + make -C tests/ check > RTLA=$(RTLA) BPFTOOL=$(SYSTEM_BPFTOOL) prove -o -f -v tests/ > + > examples: example/timerlat_bpf_action.o > .PHONY: FORCE clean check The unit tests should be a separate target, so that you can run unit tests and end-to-end tests separately. Also, the directory structure should reflect that there are two types of tests. For example, in rteval, Test:Harness-based end-to-end tests (inspired by and very similar to RTLA tests) are in tests/e2e while unit tests are in tests/ directly; it might be even better to have them in tests/unit. > diff --git a/tools/tracing/rtla/tests/test_utils.c > b/tools/tracing/rtla/tests/test_utils.c > new file mode 100644 > index 000000000000..92ed49d60d33 > --- /dev/null > +++ b/tools/tracing/rtla/tests/test_utils.c > @@ -0,0 +1,74 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Unit tests for src/utils.c parsing functions > + */ > + > +#define _GNU_SOURCE > +#include <check.h> > +#include <unistd.h> > + > +#include "../src/utils.h" > + Here, you add an external dependency on libcheck. That has to be documented in both the commit message and README, and it should also be added to tools/build dependency detection (in a separate commit) first before being added to RTLA, so that RTLA can check for the dependency and disable unit tests if it is not present. > diff --git a/tools/tracing/rtla/tests/Makefile > b/tools/tracing/rtla/tests/Makefile > new file mode 100644 > index 000000000000..fe187306a404 > --- /dev/null > +++ b/tools/tracing/rtla/tests/Makefile > @@ -0,0 +1,12 @@ > +LIBS := -lcheck > + > +test_utils: test_utils.c ../src/utils.c > + $(CC) $(CFLAGS) -o $@ $^ $(LIBS) > + > +check: test_utils > + ./test_utils > + > +clean: > + rm -f test_utils > + > +.PHONY: check clean Is there a reason for doing a separate Makefile for the tests? That will require explicitly passing variables to the child make during the build. It appears to me that it would be more natural to just include unit tests into the main Makefile (or a file included from it, e.g. Makefile.test), which already covers end-to-end tests. That would also allow it to use RTLA build system's dependency detection. Thanks, Tomas
