[PATCH] tools/nolibc: remove LINUX_REBOOT_ constants

[PATCH] tools/nolibc: remove LINUX_REBOOT_ constants

[Thomas Weißschuh]

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.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/types.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index aedd7d9e3f64..15b0baffd336 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -86,14 +86,6 @@
 #define SEEK_CUR       1
 #define SEEK_END       2
 
-/* cmd for reboot() */
-#define LINUX_REBOOT_MAGIC1         0xfee1dead
-#define LINUX_REBOOT_MAGIC2         0x28121969
-#define LINUX_REBOOT_CMD_HALT       0xcdef0123
-#define LINUX_REBOOT_CMD_POWER_OFF  0x4321fedc
-#define LINUX_REBOOT_CMD_RESTART    0x01234567
-#define LINUX_REBOOT_CMD_SW_SUSPEND 0xd000fce2
-
 /* Macros used on waitpid()'s return status */
 #define WEXITSTATUS(status) (((status) & 0xff00) >> 8)
 #define WIFEXITED(status)   (((status) & 0x7f) == 0)

---
base-commit: 33afd4b76393627477e878b3b195d606e585d816
change-id: 20230428-nolibc-reboot-719e6b06d80c

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

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

[Willy Tarreau]

Hi Thomas,

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:

  In file included from /g/public/linux/master/tools/include/nolibc/nolibc.h:99,
                   from <command-line>:
  /g/public/linux/master/tools/include/nolibc/sys.h: In function 'reboot':
  /g/public/linux/master/tools/include/nolibc/sys.h:972:30: error: 'LINUX_REBOOT_MAGIC1' undeclared (first use in this function)
    972 |         int ret = sys_reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, cmd, 0);
        |                              ^~~~~~~~~~~~~~~~~~~
  /g/public/linux/master/tools/include/nolibc/sys.h:972:30: note: each undeclared identifier is reported only once for each function it appears in

I suspect it might be like the S_* macros for stat() that we had to
guard against. What build conflict did you meet ? I would like as well
to redefine the least possible and if we can make sure to fix the
conflict efficiently without breaking code, that would be better.

Thanks,
Willy

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

[Thomas Weißschuh]

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.

>   In file included from /g/public/linux/master/tools/include/nolibc/nolibc.h:99,
>                    from <command-line>:
>   /g/public/linux/master/tools/include/nolibc/sys.h: In function 'reboot':
>   /g/public/linux/master/tools/include/nolibc/sys.h:972:30: error: 'LINUX_REBOOT_MAGIC1' undeclared (first use in this function)
>     972 |         int ret = sys_reboot(LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2, cmd, 0);
>         |                              ^~~~~~~~~~~~~~~~~~~
>   /g/public/linux/master/tools/include/nolibc/sys.h:972:30: note: each undeclared identifier is reported only once for each function it appears in
> 
> I suspect it might be like the S_* macros for stat() that we had to
> guard against. What build conflict did you meet ? I would like as well
> to redefine the least possible and if we can make sure to fix the
> conflict efficiently without breaking code, that would be better.

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"


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

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 

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

[Thomas Weißschuh]

On 2023-05-02 08:59:27+0200, Willy Tarreau wrote:

<snip>

> > 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 ;-)

This is absolutely not urgent. 6.5 is fine.

Thomas

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

[Willy Tarreau]

On Tue, May 02, 2023 at 09:05:29AM +0200, Thomas Weißschuh wrote:
> On 2023-05-02 08:59:27+0200, Willy Tarreau wrote:
> 
> <snip>
> 
> > > 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 ;-)
> 
> This is absolutely not urgent. 6.5 is fine.

OK, now queued on top of my 20230415-nolibc-updates-4a branch that
I'll soon send to Paul.

Thank you!
Willy