[PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set

Benjamin Berg posted 11 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Benjamin Berg 1 week, 5 days ago
From: Benjamin Berg <benjamin.berg@intel.com>

There is no errno variable when NOLIBC_IGNORE_ERRNO is defined. As such,
the perror function does not make any sense then and cannot compile.

Fixes: acab7bcdb1bc ("tools/nolibc/stdio: add perror() to report the errno value")
Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
Acked-by: Thomas Weißschuh <linux@weissschuh.net>
---
 tools/include/nolibc/stdio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index 7630234408c5..c512159b8374 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -597,11 +597,13 @@ int sscanf(const char *str, const char *format, ...)
 	return ret;
 }
 
+#ifndef NOLIBC_IGNORE_ERRNO
 static __attribute__((unused))
 void perror(const char *msg)
 {
 	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
 }
+#endif
 
 static __attribute__((unused))
 int setvbuf(FILE *stream __attribute__((unused)),
-- 
2.51.0

Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Willy Tarreau 1 week, 3 days ago
Hi Benjamin,

On Fri, Sep 19, 2025 at 05:34:12PM +0200, Benjamin Berg wrote:
> From: Benjamin Berg <benjamin.berg@intel.com>
> 
> There is no errno variable when NOLIBC_IGNORE_ERRNO is defined. As such,
> the perror function does not make any sense then and cannot compile.
> 
> Fixes: acab7bcdb1bc ("tools/nolibc/stdio: add perror() to report the errno value")
> Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> Acked-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  tools/include/nolibc/stdio.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 7630234408c5..c512159b8374 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
> @@ -597,11 +597,13 @@ int sscanf(const char *str, const char *format, ...)
>  	return ret;
>  }
>  
> +#ifndef NOLIBC_IGNORE_ERRNO
>  static __attribute__((unused))
>  void perror(const char *msg)
>  {
>  	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
>  }
> +#endif

Please instead place the ifndef inside the function so that code calling
perror() continues to build. The original goal of that macro was to
further shrink programs at the expense of losing error details. But we
should be able to continue to build working programs with that macro
defined. There's nothing hard set in stone regarding this but here it's
easy to preserve a working behavior by having something like this for
example:

  static __attribute__((unused))
  void perror(const char *msg)
  {
 +#ifdef NOLIBC_IGNORE_ERRNO
 + 	fprintf(stderr, "%s\n", (msg && *msg) ? msg : "unknown error");
 +#else
  	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
 +#endif
  }

thanks!
Willy
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Thomas Weißschuh 1 week, 3 days ago
On 2025-09-21 09:55:11+0200, Willy Tarreau wrote:
> Hi Benjamin,
> 
> On Fri, Sep 19, 2025 at 05:34:12PM +0200, Benjamin Berg wrote:
> > From: Benjamin Berg <benjamin.berg@intel.com>
> > 
> > There is no errno variable when NOLIBC_IGNORE_ERRNO is defined. As such,
> > the perror function does not make any sense then and cannot compile.
> > 
> > Fixes: acab7bcdb1bc ("tools/nolibc/stdio: add perror() to report the errno value")
> > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > Acked-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  tools/include/nolibc/stdio.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > index 7630234408c5..c512159b8374 100644
> > --- a/tools/include/nolibc/stdio.h
> > +++ b/tools/include/nolibc/stdio.h
> > @@ -597,11 +597,13 @@ int sscanf(const char *str, const char *format, ...)
> >  	return ret;
> >  }
> >  
> > +#ifndef NOLIBC_IGNORE_ERRNO
> >  static __attribute__((unused))
> >  void perror(const char *msg)
> >  {
> >  	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
> >  }
> > +#endif
> 
> Please instead place the ifndef inside the function so that code calling
> perror() continues to build. The original goal of that macro was to
> further shrink programs at the expense of losing error details. But we
> should be able to continue to build working programs with that macro
> defined. There's nothing hard set in stone regarding this but here it's
> easy to preserve a working behavior by having something like this for
> example:
> 
>   static __attribute__((unused))
>   void perror(const char *msg)
>   {
>  +#ifdef NOLIBC_IGNORE_ERRNO
>  + 	fprintf(stderr, "%s\n", (msg && *msg) ? msg : "unknown error");
>  +#else
>   	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
>  +#endif
>   }

For the plain `errno` variable and printf(%m) we don't have such
fallbacks. With NOLIBC_IGNORE_ERRNO the compilation either fails or the
results are undefined. Personally I prefer not defining perror() here.


Thomas
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Willy Tarreau 1 week, 3 days ago
On Sun, Sep 21, 2025 at 06:37:30PM +0200, Thomas Weißschuh wrote:
> On 2025-09-21 09:55:11+0200, Willy Tarreau wrote:
> > Hi Benjamin,
> > 
> > On Fri, Sep 19, 2025 at 05:34:12PM +0200, Benjamin Berg wrote:
> > > From: Benjamin Berg <benjamin.berg@intel.com>
> > > 
> > > There is no errno variable when NOLIBC_IGNORE_ERRNO is defined. As such,
> > > the perror function does not make any sense then and cannot compile.
> > > 
> > > Fixes: acab7bcdb1bc ("tools/nolibc/stdio: add perror() to report the errno value")
> > > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > > Acked-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  tools/include/nolibc/stdio.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > > index 7630234408c5..c512159b8374 100644
> > > --- a/tools/include/nolibc/stdio.h
> > > +++ b/tools/include/nolibc/stdio.h
> > > @@ -597,11 +597,13 @@ int sscanf(const char *str, const char *format, ...)
> > >  	return ret;
> > >  }
> > >  
> > > +#ifndef NOLIBC_IGNORE_ERRNO
> > >  static __attribute__((unused))
> > >  void perror(const char *msg)
> > >  {
> > >  	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
> > >  }
> > > +#endif
> > 
> > Please instead place the ifndef inside the function so that code calling
> > perror() continues to build. The original goal of that macro was to
> > further shrink programs at the expense of losing error details. But we
> > should be able to continue to build working programs with that macro
> > defined. There's nothing hard set in stone regarding this but here it's
> > easy to preserve a working behavior by having something like this for
> > example:
> > 
> >   static __attribute__((unused))
> >   void perror(const char *msg)
> >   {
> >  +#ifdef NOLIBC_IGNORE_ERRNO
> >  + 	fprintf(stderr, "%s\n", (msg && *msg) ? msg : "unknown error");
> >  +#else
> >   	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
> >  +#endif
> >   }
> 
> For the plain `errno` variable and printf(%m) we don't have such
> fallbacks. With NOLIBC_IGNORE_ERRNO the compilation either fails or the
> results are undefined. Personally I prefer not defining perror() here.

For me it's still a problem because that breaks the original purpose and
current behavior. You cannot anymore compare the size of with/without errno
for example, like here:

   text    data     bss     dec     hex filename
  20659      24   39424   60107    eacb init
  19836      24   39424   59284    e794 init-noerrno

Perror doesn't just display the error name/number, it also prints a message
about that error that doesn't need errno.

For "%m", that's fair enough, I didn't notice that one. We could imagine
improving it by just emitting "?" or any such thing. But right now it will
indeed proceed like you describe (that's already the case, it's not changed
by this patch).

I don't want to block that patch but I'm annoyed that it unfairly blocks a
legitimate error function that normally provides sufficient context in error
paths so that errno can be ignored, such as here:

   if (open(path, O_RDONLY) < 0) {
        perror("open()");
        return -1;
   }

What's even more problematic is that Benjamin precisely fixed two other
breakage cases in the same series for the same reason that they were
blocking the build without errno, so it would be more consistent that
this one isn't newly broken.

Willy
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Benjamin Berg 1 week, 3 days ago
Hi,

On Sun, 2025-09-21 at 18:37 +0200, Thomas Weißschuh wrote:
> On 2025-09-21 09:55:11+0200, Willy Tarreau wrote:
> > Hi Benjamin,
> > 
> > On Fri, Sep 19, 2025 at 05:34:12PM +0200, Benjamin Berg wrote:
> > > From: Benjamin Berg <benjamin.berg@intel.com>
> > > 
> > > There is no errno variable when NOLIBC_IGNORE_ERRNO is defined. As such,
> > > the perror function does not make any sense then and cannot compile.
> > > 
> > > Fixes: acab7bcdb1bc ("tools/nolibc/stdio: add perror() to report the errno value")
> > > Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
> > > Acked-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  tools/include/nolibc/stdio.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > > index 7630234408c5..c512159b8374 100644
> > > --- a/tools/include/nolibc/stdio.h
> > > +++ b/tools/include/nolibc/stdio.h
> > > @@ -597,11 +597,13 @@ int sscanf(const char *str, const char *format, ...)
> > >  	return ret;
> > >  }
> > >  
> > > +#ifndef NOLIBC_IGNORE_ERRNO
> > >  static __attribute__((unused))
> > >  void perror(const char *msg)
> > >  {
> > >  	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
> > >  }
> > > +#endif
> > 
> > Please instead place the ifndef inside the function so that code calling
> > perror() continues to build. The original goal of that macro was to
> > further shrink programs at the expense of losing error details. But we
> > should be able to continue to build working programs with that macro
> > defined. There's nothing hard set in stone regarding this but here it's
> > easy to preserve a working behavior by having something like this for
> > example:
> > 
> >   static __attribute__((unused))
> >   void perror(const char *msg)
> >   {
> >  +#ifdef NOLIBC_IGNORE_ERRNO
> >  + 	fprintf(stderr, "%s\n", (msg && *msg) ? msg : "unknown error");
> >  +#else
> >   	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
> >  +#endif
> >   }
> 
> For the plain `errno` variable and printf(%m) we don't have such
> fallbacks. With NOLIBC_IGNORE_ERRNO the compilation either fails or the
> results are undefined. Personally I prefer not defining perror() here.

So, with NOLIBC_IGNORE_ERRNO, we do not have the "errno" variable
either and code using it will break. I actually think that this is a
good thing and it is part of the reason that I wanted to explicitly set
the flag for UML.

This also ties to the question of the other mail. I prefer "errno" not
to be available if it is not actually safe to use. UML does use threads
in some places (and may use it extensively in the future). The current
"errno" implementation is not threadsafe and I see neither an obvious
way nor a need to change that. By setting NOLIBC_IGNORE_ERRNO any
unsafe code will not compile and can be changed to use the sys_*
functions to avoid errno.

Benjamin
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Willy Tarreau 1 week, 3 days ago
On Sun, Sep 21, 2025 at 07:05:24PM +0200, Benjamin Berg wrote:
> This also ties to the question of the other mail. I prefer "errno" not
> to be available if it is not actually safe to use. UML does use threads
> in some places (and may use it extensively in the future). The current
> "errno" implementation is not threadsafe and I see neither an obvious
> way nor a need to change that. By setting NOLIBC_IGNORE_ERRNO any
> unsafe code will not compile and can be changed to use the sys_*
> functions to avoid errno.

That's the point I disagree with because here we're not using errno
more than printf() or dirent(). Why fix dirent() to build without errno
and break perror() ? Why not also break printf() then ? All of this must
be consistent. We're unbreaking some arbitrary functions and breaking
other arbitrary ones, that's not logical.

I'm totally fine with saying that errno shouldn't be defined when building
without errno, but all functions must continue to be defined. perror() is
used to print an error message, it's a valid use case just as printf() and
should remain.

If we disable perror for this, then we must also disable usage of printf
for consistency (and I don't want this either).

Willy
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Thomas Weißschuh 1 week, 3 days ago
On 2025-09-21 19:13:23+0200, Willy Tarreau wrote:
> On Sun, Sep 21, 2025 at 07:05:24PM +0200, Benjamin Berg wrote:
> > This also ties to the question of the other mail. I prefer "errno" not
> > to be available if it is not actually safe to use. UML does use threads
> > in some places (and may use it extensively in the future). The current
> > "errno" implementation is not threadsafe and I see neither an obvious
> > way nor a need to change that. By setting NOLIBC_IGNORE_ERRNO any
> > unsafe code will not compile and can be changed to use the sys_*
> > functions to avoid errno.
> 
> That's the point I disagree with because here we're not using errno
> more than printf() or dirent(). Why fix dirent() to build without errno
> and break perror() ? Why not also break printf() then ? All of this must
> be consistent. We're unbreaking some arbitrary functions and breaking
> other arbitrary ones, that's not logical.

printf() is already broken. So by breaking perror() it would have been
consistent :-/.

> I'm totally fine with saying that errno shouldn't be defined when building
> without errno, but all functions must continue to be defined. perror() is
> used to print an error message, it's a valid use case just as printf() and
> should remain.
> 
> If we disable perror for this, then we must also disable usage of printf
> for consistency (and I don't want this either).

Then let's also fix printf(). Benjamin, do you want to add this to your
series? It should be consitent with the perror() fallback.


Thomas
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Willy Tarreau 1 week, 3 days ago
On Sun, Sep 21, 2025 at 08:26:35PM +0200, Thomas Weißschuh wrote:
> > I'm totally fine with saying that errno shouldn't be defined when building
> > without errno, but all functions must continue to be defined. perror() is
> > used to print an error message, it's a valid use case just as printf() and
> > should remain.
> > 
> > If we disable perror for this, then we must also disable usage of printf
> > for consistency (and I don't want this either).
> 
> Then let's also fix printf(). Benjamin, do you want to add this to your
> series? It should be consitent with the perror() fallback.

Yes that would be great given that the series focuses on fixing errno
usage.

Thanks,
Willy
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Benjamin Berg 1 week, 3 days ago
Hi,

On Sun, 2025-09-21 at 19:13 +0200, Willy Tarreau wrote:
> On Sun, Sep 21, 2025 at 07:05:24PM +0200, Benjamin Berg wrote:
> > This also ties to the question of the other mail. I prefer "errno" not
> > to be available if it is not actually safe to use. UML does use threads
> > in some places (and may use it extensively in the future). The current
> > "errno" implementation is not threadsafe and I see neither an obvious
> > way nor a need to change that. By setting NOLIBC_IGNORE_ERRNO any
> > unsafe code will not compile and can be changed to use the sys_*
> > functions to avoid errno.
> 
> That's the point I disagree with because here we're not using errno
> more than printf() or dirent(). Why fix dirent() to build without errno
> and break perror() ? Why not also break printf() then ? All of this must
> be consistent. We're unbreaking some arbitrary functions and breaking
> other arbitrary ones, that's not logical.
> 
> I'm totally fine with saying that errno shouldn't be defined when building
> without errno, but all functions must continue to be defined. perror() is
> used to print an error message, it's a valid use case just as printf() and
> should remain.
> 
> If we disable perror for this, then we must also disable usage of printf
> for consistency (and I don't want this either).

Right, fair enough. It is true that it does not really hurt to keep
perror defined. I doubt there is much code out there, but I also don't
really have a a strong argument against keeping perror. After all, it
will "just" result in a bad error messages rather than undefined
behaviour.

Benjamin
Re: [PATCH v2 03/11] tools/nolibc/stdio: remove perror if NOLIBC_IGNORE_ERRNO is set
Posted by Willy Tarreau 1 week, 3 days ago
On Sun, Sep 21, 2025 at 07:16:50PM +0200, Benjamin Berg wrote:
> Hi,
> 
> On Sun, 2025-09-21 at 19:13 +0200, Willy Tarreau wrote:
> > On Sun, Sep 21, 2025 at 07:05:24PM +0200, Benjamin Berg wrote:
> > > This also ties to the question of the other mail. I prefer "errno" not
> > > to be available if it is not actually safe to use. UML does use threads
> > > in some places (and may use it extensively in the future). The current
> > > "errno" implementation is not threadsafe and I see neither an obvious
> > > way nor a need to change that. By setting NOLIBC_IGNORE_ERRNO any
> > > unsafe code will not compile and can be changed to use the sys_*
> > > functions to avoid errno.
> > 
> > That's the point I disagree with because here we're not using errno
> > more than printf() or dirent(). Why fix dirent() to build without errno
> > and break perror() ? Why not also break printf() then ? All of this must
> > be consistent. We're unbreaking some arbitrary functions and breaking
> > other arbitrary ones, that's not logical.
> > 
> > I'm totally fine with saying that errno shouldn't be defined when building
> > without errno, but all functions must continue to be defined. perror() is
> > used to print an error message, it's a valid use case just as printf() and
> > should remain.
> > 
> > If we disable perror for this, then we must also disable usage of printf
> > for consistency (and I don't want this either).
> 
> Right, fair enough. It is true that it does not really hurt to keep
> perror defined. I doubt there is much code out there, but I also don't
> really have a a strong argument against keeping perror. After all, it
> will "just" result in a bad error messages rather than undefined
> behaviour.

It shouldn't be a "bad" error message, just a limited one, which is the
main purpose of ignoring errno (i.e. where we're running we don't care
about error details since the user only needs to know that it failed, or
may even not know about it at all). perror *does* display the caller's
error message. I'm personally fine with seeing:

    "open(): unknown error"

for:

    if (open(path, O_RDONLY) < 0)
         perror("open()");

when building without errno.

Willy