Re: [PATCH for-4.22 v2 4/5] tests/numa: add unit tests for NUMA setup logic

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Jun 03, 2026 at 09:18:35PM +0200, Roger Pau Monne wrote:
> diff --git a/tools/tests/numa/Makefile b/tools/tests/numa/Makefile
> new file mode 100644
> index 000000000000..5235f9d6297f
> --- /dev/null
> +++ b/tools/tests/numa/Makefile
> +
> +.PHONY: uninstall
> +uninstall:
> +     $(RM) -- $(patsubst %,$(DESTDIR)$(LIBEXEC)/tests/%,$(TARGETS))

There's a simpler way to write this, with
    $(addprefix $(DESTDIR)$(LIBEXEC)/tests/,$(TARGETS))
But that's ok to.

> +
> +numa.h: $(XEN_ROOT)/xen/include/xen/numa.h
> +     sed -e '/^#[[:space:]]*include/d' <$< >$@
> +
> +CFLAGS += -D__XEN_TOOLS__
> +CFLAGS += $(APPEND_CFLAGS)
> +CFLAGS += $(CFLAGS_xeninclude)
> +
> +test-numa: test-numa.c numa.h
> +     $(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_$*.o) -o $@ $< $(APPEND_CFLAGS)

$* should be undefined here.
So we have $(CFLAGS_.o), but that variable doesn't exit either.
You could remove $(CFLAGS_$*.o), it's not use here.

Also, $(APPEND_CFLAGS) is added twice, once via $(CFLAGS) and a second
time on the command line. I think the one added to $(CFLAGS) should be
removed.

> diff --git a/tools/tests/numa/test-numa.c b/tools/tests/numa/test-numa.c
> new file mode 100644
> index 000000000000..bced68d4d7f1
> --- /dev/null
> +++ b/tools/tests/numa/test-numa.c
> +
> +        for ( j = 0;
> +              j < ARRAY_SIZE(tests[i].affinity) && tests[i].affinity[j].end;

Why do you test the value `.end` ? ARRAY_SIZE is likely enough as the
test array is static. Same thing later, with the `ram` array.


Anyway, it's all look good enough to me:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech