Re: [PATCH] tools/nolibc: remove LINUX_REBOOT_ constants

[Willy Tarreau]

On Tue, May 02, 2023 at 08:47:15AM +0200, Thomas Weißschuh wrote:
> On 2023-05-02 08:32:15+0200, Willy Tarreau wrote:
> > On Fri, Apr 28, 2023 at 05:52:11PM +0200, Thomas Weißschuh wrote:
> > > The same constants and some more have been exposed to userspace via
> > > linux/reboot.h for a long time.
> > > 
> > > To avoid conflicts and trim down nolibc a bit drop the custom
> > > definitions.
> > 
> > For me it breaks the build when including nolibc directly, so most
> > likely we need to include certain files:
> 
> Indeed, sorry no idea how I missed that.

No worries, it happens to me as well and that's the benefit of
cross-testing ;-)

> The conflict looks like this:
> 
>     In file included from nolibc-test.c:18:
>     sysroot/x86/include/linux/reboot.h:10: warning: "LINUX_REBOOT_MAGIC2" redefined
>        10 | #define LINUX_REBOOT_MAGIC2     672274793
>           | 
>     In file included from sysroot/x86/include/nolibc.h:98,
>                      from sysroot/x86/include/errno.h:26,
>                      from sysroot/x86/include/stdio.h:14,
>                      from nolibc-test.c:15:
>     ... and all the other ones.
> 
> 
> 
> The following trivial fix on top of my patch would fix the problem:
> 
> diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> index 5d624dc63a42..9d27131c224e 100644
> --- a/tools/include/nolibc/sys.h
> +++ b/tools/include/nolibc/sys.h
> @@ -21,6 +21,7 @@
>  #include <linux/auxvec.h>
>  #include <linux/fcntl.h> // for O_* and AT_*
>  #include <linux/stat.h>  // for statx()
> +#include <linux/reboot.h> // for LINUX_REBOOT_*
>  
>  #include "arch.h"
>  #include "errno.h"

Indeed it works for me as well.

> Want me to send a v2 or will you fix it up on your side?

It depends. If for you it's a fix and needed for 6.4 (or maybe older),
then that one is needed with the "//" comment, and it will later
conflict with your previous cleanup patch that's already queued. If
you're fine with having it queued for 6.5 only however, then I'll just
edit your patch and add that above. I tend to think the second solution
is sufficient given that nobody complained till now ;-)

Just let me know,
thanks,
Willy