| Message ID | 20230926071432.51866-2-knowak@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix cross-compilation issue with randbytes | expand |
On Tue, Sep 26, 2023 at 09:14:32AM +0200, Krzesimir Nowak wrote: > randbytes.c was mostly split off from crc32.c and, like crc32.c, is > used for selftests, which are run on the build host. As such it should > not include platform_defs.h which in turn includes urcu.h from > userspace-rcu library, because the build host might not have the > library installed. Why not get rid of the build host crc32c selftest? It's not that useful for cross-compiling and nowadays mkfs.xfs and xfs_repair have their own builtin selftests. Anyone messing with xfsprogs should be running fstests (in addition to the maintainers) so I don't really see the point of running crc32cselftest on the *build* host. (Carlos: any thoughts on this?) --D > Signed-off-by: Krzesimir Nowak <knowak@microsoft.com> > --- > libfrog/randbytes.c | 1 - > libfrog/randbytes.h | 2 ++ > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libfrog/randbytes.c b/libfrog/randbytes.c > index f22da0d3..2023b601 100644 > --- a/libfrog/randbytes.c > +++ b/libfrog/randbytes.c > @@ -6,7 +6,6 @@ > * > * This is the buffer of random bytes used for self tests. > */ > -#include "platform_defs.h" > #include "libfrog/randbytes.h" > > /* 4096 random bytes */ > diff --git a/libfrog/randbytes.h b/libfrog/randbytes.h > index 00fd7c4c..fddea9c7 100644 > --- a/libfrog/randbytes.h > +++ b/libfrog/randbytes.h > @@ -6,6 +6,8 @@ > #ifndef __LIBFROG_RANDBYTES_H__ > #define __LIBFROG_RANDBYTES_H__ > > +#include <stdint.h> > + > extern uint8_t randbytes_test_buf[]; > > #endif /* __LIBFROG_RANDBYTES_H__ */ > -- > 2.25.1 >
On Tue, Sep 26, 2023 at 07:41:00AM -0700, Darrick J. Wong wrote: > On Tue, Sep 26, 2023 at 09:14:32AM +0200, Krzesimir Nowak wrote: > > randbytes.c was mostly split off from crc32.c and, like crc32.c, is > > used for selftests, which are run on the build host. As such it should > > not include platform_defs.h which in turn includes urcu.h from > > userspace-rcu library, because the build host might not have the > > library installed. > > Why not get rid of the build host crc32c selftest? It's not that useful > for cross-compiling and nowadays mkfs.xfs and xfs_repair have their own > builtin selftests. Anyone messing with xfsprogs should be running > fstests (in addition to the maintainers) so I don't really see the point > of running crc32cselftest on the *build* host. Agreed. Running the test on the build-host served a purpose a decade ago when we first added crc support to the on-disk format. I don't think it is necessary to do this anymore... -Dave.
On Tue, Sep 26, 2023 at 07:41:00AM -0700, Darrick J. Wong wrote: > On Tue, Sep 26, 2023 at 09:14:32AM +0200, Krzesimir Nowak wrote: > > randbytes.c was mostly split off from crc32.c and, like crc32.c, is > > used for selftests, which are run on the build host. As such it should > > not include platform_defs.h which in turn includes urcu.h from > > userspace-rcu library, because the build host might not have the > > library installed. > > Why not get rid of the build host crc32c selftest? It's not that useful > for cross-compiling and nowadays mkfs.xfs and xfs_repair have their own > builtin selftests. Anyone messing with xfsprogs should be running > fstests (in addition to the maintainers) so I don't really see the point > of running crc32cselftest on the *build* host. > > (Carlos: any thoughts on this?) /me back from holidays... Yeah, sounds reasonable, IMO crc32selftest can go. Carlos > > --D > > > Signed-off-by: Krzesimir Nowak <knowak@microsoft.com> > > --- > > libfrog/randbytes.c | 1 - > > libfrog/randbytes.h | 2 ++ > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libfrog/randbytes.c b/libfrog/randbytes.c > > index f22da0d3..2023b601 100644 > > --- a/libfrog/randbytes.c > > +++ b/libfrog/randbytes.c > > @@ -6,7 +6,6 @@ > > * > > * This is the buffer of random bytes used for self tests. > > */ > > -#include "platform_defs.h" > > #include "libfrog/randbytes.h" > > > > /* 4096 random bytes */ > > diff --git a/libfrog/randbytes.h b/libfrog/randbytes.h > > index 00fd7c4c..fddea9c7 100644 > > --- a/libfrog/randbytes.h > > +++ b/libfrog/randbytes.h > > @@ -6,6 +6,8 @@ > > #ifndef __LIBFROG_RANDBYTES_H__ > > #define __LIBFROG_RANDBYTES_H__ > > > > +#include <stdint.h> > > + > > extern uint8_t randbytes_test_buf[]; > > > > #endif /* __LIBFROG_RANDBYTES_H__ */ > > -- > > 2.25.1 > >
wt., 3 paź 2023 o 13:15 Carlos Maiolino <cem@kernel.org> napisał(a): > > On Tue, Sep 26, 2023 at 07:41:00AM -0700, Darrick J. Wong wrote: > > On Tue, Sep 26, 2023 at 09:14:32AM +0200, Krzesimir Nowak wrote: > > > randbytes.c was mostly split off from crc32.c and, like crc32.c, is > > > used for selftests, which are run on the build host. As such it should > > > not include platform_defs.h which in turn includes urcu.h from > > > userspace-rcu library, because the build host might not have the > > > library installed. > > > > Why not get rid of the build host crc32c selftest? It's not that useful > > for cross-compiling and nowadays mkfs.xfs and xfs_repair have their own > > builtin selftests. Anyone messing with xfsprogs should be running > > fstests (in addition to the maintainers) so I don't really see the point > > of running crc32cselftest on the *build* host. > > > > (Carlos: any thoughts on this?) > > /me back from holidays... > > Yeah, sounds reasonable, IMO crc32selftest can go. Hi, There's a follow-up patch that I have sent - it drops the crc32 selftest: https://marc.info/?l=linux-xfs&m=169590021227419&w=2 Not sure if there is anything else I should do. If so, please let me know. Thanks! Cheers, Krzesimir > > Carlos > > > > > --D > > > > > Signed-off-by: Krzesimir Nowak <knowak@microsoft.com> > > > --- > > > libfrog/randbytes.c | 1 - > > > libfrog/randbytes.h | 2 ++ > > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/libfrog/randbytes.c b/libfrog/randbytes.c > > > index f22da0d3..2023b601 100644 > > > --- a/libfrog/randbytes.c > > > +++ b/libfrog/randbytes.c > > > @@ -6,7 +6,6 @@ > > > * > > > * This is the buffer of random bytes used for self tests. > > > */ > > > -#include "platform_defs.h" > > > #include "libfrog/randbytes.h" > > > > > > /* 4096 random bytes */ > > > diff --git a/libfrog/randbytes.h b/libfrog/randbytes.h > > > index 00fd7c4c..fddea9c7 100644 > > > --- a/libfrog/randbytes.h > > > +++ b/libfrog/randbytes.h > > > @@ -6,6 +6,8 @@ > > > #ifndef __LIBFROG_RANDBYTES_H__ > > > #define __LIBFROG_RANDBYTES_H__ > > > > > > +#include <stdint.h> > > > + > > > extern uint8_t randbytes_test_buf[]; > > > > > > #endif /* __LIBFROG_RANDBYTES_H__ */ > > > -- > > > 2.25.1 > > >
diff --git a/libfrog/randbytes.c b/libfrog/randbytes.c index f22da0d3..2023b601 100644 --- a/libfrog/randbytes.c +++ b/libfrog/randbytes.c @@ -6,7 +6,6 @@ * * This is the buffer of random bytes used for self tests. */ -#include "platform_defs.h" #include "libfrog/randbytes.h" /* 4096 random bytes */ diff --git a/libfrog/randbytes.h b/libfrog/randbytes.h index 00fd7c4c..fddea9c7 100644 --- a/libfrog/randbytes.h +++ b/libfrog/randbytes.h @@ -6,6 +6,8 @@ #ifndef __LIBFROG_RANDBYTES_H__ #define __LIBFROG_RANDBYTES_H__ +#include <stdint.h> + extern uint8_t randbytes_test_buf[]; #endif /* __LIBFROG_RANDBYTES_H__ */
randbytes.c was mostly split off from crc32.c and, like crc32.c, is used for selftests, which are run on the build host. As such it should not include platform_defs.h which in turn includes urcu.h from userspace-rcu library, because the build host might not have the library installed. Signed-off-by: Krzesimir Nowak <knowak@microsoft.com> --- libfrog/randbytes.c | 1 - libfrog/randbytes.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-)