Re: [LTP] [PATCH v1 01/10] Add SAFE_STATX macro

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> Hi,

...
> > > > > +++ b/include/tst_safe_macros.h
> > > > > @@ -503,4 +503,11 @@ int safe_sscanf(const char *file, const int lineno, const char *restrict buffer,
> > > > >    #define SAFE_SSCANF(buffer, format, ...) \
> > > > >    	safe_sscanf(__FILE__, __LINE__, (buffer), (format),	##__VA_ARGS__)
> > > > > +struct statx;
> > > > Could you please remove this? (unneeded)
> > > That's needed because in some distro statx is not defined before reaching
> > > that line causing build failure.
> > <sys/stat.h> are included in lapi/stat.h. I wonder if <linux/stat.h> would fail.
> It's related with distros which need to use fallback. There's no fallback of
> "struct statx" when defining the
> statx() syscall wrapper, so it fails during build.

OK, struct statx is at least on musl guarded with _GNU_SOURCE, that's why
struct statx in the header helps. This is better than define _GNU_SOURCE also
for the header (it's in tst_safe_macros.c). But it would be good to document
this (either in the commit message or in the source) - I always wonder when
staring in various workarounds like this few years later and wondering if it can
be removed.

I was thinking it would be possible to switch to <linux/stat.h> and we would
save the detection. But given struct statx is used in tst_safe_macros.h,
we would need to replace all <sys/stat.h> with <linux/stat.h> also in the tests
which use tst_safe_macros.h:

$ git grep -l tst_safe_macros.h $(git grep -l -e include..lapi/stat.h -e include..sys/stat.h) | wc -l
8

But I don't think it's a good idea to switch to <linux/stat.h>.

Kind regards,
Petr

> > If the definition later works it should be fixable by including the needed
> > header in lapi/stat.h, right?

> > Could you post link to CI job which failed?

> > Kind regards,
> > Petr

> > > > With that, you might add for this patch in the next version:
> > > > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > > > Kind regards,
> > > > Petr
> > > Andrea

> Andrea

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp