Re: [PATCH v2 6/8] test/pdx: add PDX compression unit tests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jun 24, 2025 at 03:37:34PM +0200, Anthony PERARD wrote:
> On Fri, Jun 20, 2025 at 01:11:28PM +0200, Roger Pau Monne wrote:
> > +.PHONY: run
> > +run: $(TARGETS)
> > +ifeq ($(CC),$(HOSTCC))
> > +   for test in $? ; do \
> > +           ./$$test ;  \
> > +   done
> 
> You need to add `set -e` or the exit value from the tested binary might
> be ignored. This `run` target only failed if the last test binary return
> a failure.

Oh, I did see it failing, but both tests at teh same time, that's why
I didn't notice that only the first failing won't be reported.

> > +else
> > +   $(warning HOSTCC != CC, will not run test)
> > +endif
> > +
> > +.PHONY: clean
> > +clean:
> > +   $(RM) -- *.o $(TARGETS) $(DEPS_RM) pdx.c pdx.h
> 
> Is this "pdx.c" left over from version? It doesn't seems to be generated
> by this makefile.

Yeah, it's a leftover from the previous version where I was also
making a local copy of pdx.c.

> > +
> > +pdx.h: $(XEN_ROOT)/xen/include/xen/pdx.h
> > +   sed -E -e '/^#[[:space:]]?include/d' <$< >$@
> 
> Why allow only zero or one space characters between '#' and
> "include"? Why not used '*' instead of '?' ?

Because that's all we currently use in the header, but I can indeed
switch to * just in case we gain includes with more spaces.

Thanks, Roger.