| Message ID | 20240515093349.7347-2-andrea.cervesato@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | statmount/listmount testing suites | expand |
Hi Andrea, could you please fix these? testcases/kernel/syscalls/statmount/statmount03.c: useless tag: format_device testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount04.c: useless tag: format_device testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount05.c: useless tag: format_device testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir testcases/kernel/syscalls/statmount/statmount06.c: useless tag: format_device > diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h > index 8de8ef106..43ff50a33 100644 > --- a/include/tst_safe_macros.h > +++ 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) With that, you might add for this patch in the next version: Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi, On 5/16/24 03:30, Petr Vorel wrote: > Hi Andrea, > > could you please fix these? Sure > > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: format_device > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: format_device > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: format_device > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > testcases/kernel/syscalls/statmount/statmount06.c: useless tag: format_device > >> diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h >> index 8de8ef106..43ff50a33 100644 >> --- a/include/tst_safe_macros.h >> +++ 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. > > With that, you might add for this patch in the next version: > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > Kind regards, > Petr Andrea
> Hi, > On 5/16/24 03:30, Petr Vorel wrote: > > Hi Andrea, > > could you please fix these? > Sure > > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: format_device > > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: format_device > > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: format_device > > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir > > testcases/kernel/syscalls/statmount/statmount06.c: useless tag: format_device > > > diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h > > > index 8de8ef106..43ff50a33 100644 > > > --- a/include/tst_safe_macros.h > > > +++ 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. 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
Hi, On 5/16/24 16:49, Petr Vorel wrote: >> Hi, >> On 5/16/24 03:30, Petr Vorel wrote: >>> Hi Andrea, >>> could you please fix these? >> Sure >>> testcases/kernel/syscalls/statmount/statmount03.c: useless tag: format_device >>> testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount03.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount04.c: useless tag: format_device >>> testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount04.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount05.c: useless tag: format_device >>> testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount05.c: useless tag: needs_tmpdir >>> testcases/kernel/syscalls/statmount/statmount06.c: useless tag: format_device >>>> diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h >>>> index 8de8ef106..43ff50a33 100644 >>>> --- a/include/tst_safe_macros.h >>>> +++ 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. > > 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
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
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 8de8ef106..43ff50a33 100644 --- a/include/tst_safe_macros.h +++ 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; +int safe_statx(const char *file, const int lineno, + int dirfd, const char *pathname, int flags, unsigned int mask, + struct statx *buf); +#define SAFE_STATX(dirfd, pathname, flags, mask, buf) \ + safe_statx(__FILE__, __LINE__, (dirfd), (pathname), (flags), (mask), (buf)) + #endif /* TST_SAFE_MACROS_H__ */ diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c index 39b8cc924..bf2f3c0d8 100644 --- a/lib/tst_safe_macros.c +++ b/lib/tst_safe_macros.c @@ -710,3 +710,24 @@ int safe_mprotect(const char *file, const int lineno, return rval; } + +int safe_statx(const char *file, const int lineno, + int dirfd, const char *pathname, int flags, unsigned int mask, + struct statx *buf) +{ + int rval; + + rval = statx(dirfd, pathname, flags, mask, buf); + + if (rval == -1) { + tst_brk_(file, lineno, TBROK | TERRNO, + "statx(%d,%s,%d,%u,%p) failed", dirfd, pathname, flags, mask, buf); + } else if (rval) { + tst_brk_(file, lineno, TBROK | TERRNO, + "Invalid statx(%d,%s,%d,%u,%p) return value %d", + dirfd, pathname, flags, mask, buf, + rval); + } + + return rval; +}