[Buildroot] package/coreutils mkfifo -m exit status

[Buildroot] package/coreutils mkfifo -m exit status

[Max Filippov]

Hello,

I've noticed that when I run
mkfifo -m 600 fifo-name
from package/coreutils on a linux system it always indicates an error
in the exit code.

Debugging showed that lchmod function call at
https://github.com/coreutils/coreutils/blob/v9.0/src/mkfifo.c#L178
returns error because when the package is configured with
gl_cv_func_fstatat_zero_flag=no the following fstatat code in lchmod
https://github.com/coreutils/gnulib/blob/9aca7b67333fd0a90bf56104325ce96199644451/lib/lchmod.c#L65
always returns error with EINVAL in errno.

Removing gl_cv_func_fstatat_zero_flag=no from the package/coreutils
config environment fixes that issue for me, but I couldn't find any
explanation as to why it was added. I can send a patch if this
is an acceptable fix, please let me know.

-- 
Thanks.
-- Max
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] package/coreutils mkfifo -m exit status

[Thomas Petazzoni]

Hello Max,

On Sun, 26 Dec 2021 01:51:47 -0800
Max Filippov <jcmvbkbc@gmail.com> wrote:

> I've noticed that when I run
> mkfifo -m 600 fifo-name
> from package/coreutils on a linux system it always indicates an error
> in the exit code.
> 
> Debugging showed that lchmod function call at
> https://github.com/coreutils/coreutils/blob/v9.0/src/mkfifo.c#L178
> returns error because when the package is configured with
> gl_cv_func_fstatat_zero_flag=no the following fstatat code in lchmod
> https://github.com/coreutils/gnulib/blob/9aca7b67333fd0a90bf56104325ce96199644451/lib/lchmod.c#L65
> always returns error with EINVAL in errno.
> 
> Removing gl_cv_func_fstatat_zero_flag=no from the package/coreutils
> config environment fixes that issue for me, but I couldn't find any
> explanation as to why it was added. I can send a patch if this
> is an acceptable fix, please let me know.

I took my extreme situation caving equipment, and went into the deep
dark cave of the gnulib code.

In the dark dark age of commit 531e3287a33c005edeeba10b0ab0910c6686e049
(https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=531e3287a33c005edeeba10b0ab0910c6686e049)
this gl_cv_func_fstatat_zero_flag was added to handle an obscure bug on
AIX 7.1, where fstatat() would return an error if the flags argument is
zero.

An AC_RUN_IFELSE() check was verifying this, and setting this
gl_cv_func_fstatat_zero_flag variable was necessary to make ./configure
pass. However setting it to "no" was a mistake, because it did imply
that yes fstatat() was broken with a zero flag, which was never the
case on Linux.

A bit later in commit c098d6b8a714de15c81ee72f86f9b7c268554682
(https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c098d6b8a714de15c81ee72f86f9b7c268554682),
this logic was improved so that setting gl_cv_func_fstatat_zero_flag
was no longer needed when cross-compiling: it would automatically
assume the bug is present.

Further later, in commit 24a579b40979e51868e07b56ea307c1b2c12bcff
(https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=24a579b40979e51868e07b56ea307c1b2c12bcff),
this was improved even more to only assume that fstatat() was broken on
AIX systems when cross-compiling, making the right guess automatically.

So when we force gl_cv_func_fstatat_zero_flag=no in Buildroot today, we
prevent HAVE_WORKING_FSTATAT_ZERO_FLAG from being set.

So, clearly dropping gl_cv_func_fstatat_zero_flag=no from Buildroot's
coreutils.mk is the right thing to do.

However, I have not been able to understand why
gl_cv_func_fstatat_zero_flag=no makes fstatat() always return an error.
The code in lib/fstatat.c is convoluted, and even looking at its Git
history, I've not been able to spot what happened.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] package/coreutils mkfifo -m exit status

[Max Filippov]

Hi Thomas,

On Sun, Dec 26, 2021 at 3:23 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> So, clearly dropping gl_cv_func_fstatat_zero_flag=no from Buildroot's
> coreutils.mk is the right thing to do.

Thanks for the confirmation, I've posted the patch.

> However, I have not been able to understand why
> gl_cv_func_fstatat_zero_flag=no makes fstatat() always return an error.
> The code in lib/fstatat.c is convoluted, and even looking at its Git
> history, I've not been able to spot what happened.

I had to look at the preprocessed source to understand it,
in the presence of gl_cv_func_fstatat_zero_flag=no it looks like this:

int rpl_fstatat(int fd, char const *file , struct stat *st, int flag)
{
 if (flag & ~0x100) {
(*__errno_location ())  =  22; return -1; };

i.e. return error with EINVAL code when flags have anything other
than AT_SYMLINK_NOFOLLOW, which is always the case for the
following call:
https://github.com/coreutils/gnulib/blob/9aca7b67333fd0a90bf56104325ce96199644451/lib/lchmod.c#L65

-- 
Thanks.
-- Max
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] package/coreutils mkfifo -m exit status

[Thomas Petazzoni]

Hello Max,

On Sun, 26 Dec 2021 23:24:53 -0800
Max Filippov <jcmvbkbc@gmail.com> wrote:

> I had to look at the preprocessed source to understand it,

Ah, good idea. Navigating the forest of ifdef-ery in gnulib land is not
trivial :/

> in the presence of gl_cv_func_fstatat_zero_flag=no it looks like this:
> 
> int rpl_fstatat(int fd, char const *file , struct stat *st, int flag)
> {
>  if (flag & ~0x100) {
> (*__errno_location ())  =  22; return -1; };
> 
> i.e. return error with EINVAL code when flags have anything other
> than AT_SYMLINK_NOFOLLOW, which is always the case for the
> following call:
> https://github.com/coreutils/gnulib/blob/9aca7b67333fd0a90bf56104325ce96199644451/lib/lchmod.c#L65

OK. This is weird though, I don't understand why they are doing this to
work-around the problem of "flags = 0" causing a failure.

But OK, fair enough, we know that gl_cv_func_fstatat_zero_flag is now
properly detected, so forcing it to "no" no longer makes sense, so I'll
go ahead and apply your patch.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot