GNU bug report logs - #50791
Recursive chmod exit status in coreutils-9.0

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50791 in the body.
You can then email your comments to 50791 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Chris Webb <chris <at> arachsys.com>
To: bug-coreutils <at> gnu.org
Subject: Recursive chmod exit status in coreutils-9.0
Date: Fri, 24 Sep 2021 22:16:16 +0100

Since bbb19b, chmod has started returning EXIT_FAILURE after skipping a
symlink during a recursive change. This affects chmod in the newly released
coreutils-9.0 vs the previous coreutils-8.32 and older versions:

  # mkdir t
  # cd t
  # : > foo
  # ln -s foo bar

  # chmod-8.32 -v -R u+r . ; echo "Exit status $?"
  mode of '.' retained as 0755 (rwxr-xr-x)
  neither symbolic link './bar' nor referent has been changed
  mode of './foo' retained as 0644 (rw-r--r--)
  Exit status 0

  # chmod-9.0 -v -R u+r . ; echo "Exit status $?"
  mode of '.' retained as 0755 (rwxr-xr-x)
  neither symbolic link './bar' nor referent has been changed
  mode of './foo' retained as 0644 (rw-r--r--)
  Exit status 1

The non-zero exit code is accompanied by no error message or diagnostic.

Success from process_file() in src/chmod.c is detected by

  ch.status >= CH_NO_CHANGE_REQUESTED

and the ordering for the possible outcomes in bbb19b puts CH_NOT_APPLIED in
the failure range. However, we finish with CH_NOT_APPLIED if and only if a
symlink has been (correctly) skipped.

I believe the simple reordering:

diff --git a/src/chmod.c b/src/chmod.c
index 37b04f500..36894854f 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -44,9 +44,9 @@ struct change_status
   enum
     {
       CH_NO_STAT,
-      CH_NOT_APPLIED,
       CH_FAILED,
       CH_NO_CHANGE_REQUESTED,
+      CH_NOT_APPLIED,
       CH_SUCCEEDED
     }
     status;

is sufficient to fix with no other side-effects.

Best wishes,
Chris.

Message #8 received at 50791 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Chris Webb <chris <at> arachsys.com>, 50791 <at> debbugs.gnu.org
Subject: Re: bug#50791: Recursive chmod exit status in coreutils-9.0
Date: Fri, 24 Sep 2021 23:50:46 +0100

forcemerge 50791 50784
stop

On 24/09/2021 22:16, Chris Webb wrote:
> Since bbb19b, chmod has started returning EXIT_FAILURE after skipping a
> symlink during a recursive change. This affects chmod in the newly released
> coreutils-9.0 vs the previous coreutils-8.32 and older versions:
> 
>    # mkdir t
>    # cd t
>    # : > foo
>    # ln -s foo bar
> 
>    # chmod-8.32 -v -R u+r . ; echo "Exit status $?"
>    mode of '.' retained as 0755 (rwxr-xr-x)
>    neither symbolic link './bar' nor referent has been changed
>    mode of './foo' retained as 0644 (rw-r--r--)
>    Exit status 0
> 
>    # chmod-9.0 -v -R u+r . ; echo "Exit status $?"
>    mode of '.' retained as 0755 (rwxr-xr-x)
>    neither symbolic link './bar' nor referent has been changed
>    mode of './foo' retained as 0644 (rw-r--r--)
>    Exit status 1
> 
> The non-zero exit code is accompanied by no error message or diagnostic.
> 
> Success from process_file() in src/chmod.c is detected by
> 
>    ch.status >= CH_NO_CHANGE_REQUESTED
> 
> and the ordering for the possible outcomes in bbb19b puts CH_NOT_APPLIED in
> the failure range. However, we finish with CH_NOT_APPLIED if and only if a
> symlink has been (correctly) skipped.
> 
> I believe the simple reordering:
> 
> diff --git a/src/chmod.c b/src/chmod.c
> index 37b04f500..36894854f 100644
> --- a/src/chmod.c
> +++ b/src/chmod.c
> @@ -44,9 +44,9 @@ struct change_status
>     enum
>       {
>         CH_NO_STAT,
> -      CH_NOT_APPLIED,
>         CH_FAILED,
>         CH_NO_CHANGE_REQUESTED,
> +      CH_NOT_APPLIED,
>         CH_SUCCEEDED
>       }
>       status;
> 
> is sufficient to fix with no other side-effects.

Thanks for the analysis and patch.
I've already pushed a very similar patch at:
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=e8b56ebd5

sorry for the trouble,
Pádraig

Message #13 received at 50791 <at> debbugs.gnu.org (full text, mbox):

From: Chris Webb <chris <at> arachsys.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 50791 <at> debbugs.gnu.org
Subject: Re: bug#50791: Recursive chmod exit status in coreutils-9.0
Date: Sat, 25 Sep 2021 07:07:56 +0100

> Thanks for the analysis and patch.
> I've already pushed a very similar patch at:
> https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=e8b56ebd5

Many thanks!

With the above fix, do we also need something like

@@ -300,7 +300,7 @@ process_file (FTS *fts, FTSENT *ent)
         describe_change (file_full_name, &ch);
     }
 
-  if (CH_NO_CHANGE_REQUESTED <= ch.status && diagnose_surprises)
+  if (CH_NOT_APPLIED <= ch.status && diagnose_surprises)
     {
       mode_t naively_expected_mode =
         mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, 0, change, NULL);

in the diagnose_surprises stanza, to avoid checking every symlink we skip for
unexpected permissions? 

Best wishes,
Chris.

Message #16 received at 50791 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Chris Webb <chris <at> arachsys.com>
Cc: 50791 <at> debbugs.gnu.org
Subject: Re: bug#50791: Recursive chmod exit status in coreutils-9.0
Date: Sat, 25 Sep 2021 11:49:06 +0100

On 25/09/2021 07:07, Chris Webb wrote:
>> Thanks for the analysis and patch.
>> I've already pushed a very similar patch at:
>> https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=e8b56ebd5
> 
> Many thanks!
> 
> With the above fix, do we also need something like
> 
> @@ -300,7 +300,7 @@ process_file (FTS *fts, FTSENT *ent)
>           describe_change (file_full_name, &ch);
>       }
>   
> -  if (CH_NO_CHANGE_REQUESTED <= ch.status && diagnose_surprises)
> +  if (CH_NOT_APPLIED <= ch.status && diagnose_surprises)
>       {
>         mode_t naively_expected_mode =
>           mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, 0, change, NULL);
> 
> in the diagnose_surprises stanza, to avoid checking every symlink we skip for
> unexpected permissions?


CH_NOT_APPLIED is smaller than CH_NO_CHANGE_REQUESTED
in the current enum ordering, so we skip this
block as expected for the ignored symlink case.

cheers,
Pádraig

Message #19 received at 50791 <at> debbugs.gnu.org (full text, mbox):

From: Chris Webb <chris <at> arachsys.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 50791 <at> debbugs.gnu.org
Subject: Re: bug#50791: Recursive chmod exit status in coreutils-9.0
Date: Sat, 25 Sep 2021 11:56:26 +0100

Pádraig Brady <P <at> draigBrady.com> wrote:

> CH_NOT_APPLIED is smaller than CH_NO_CHANGE_REQUESTED
> in the current enum ordering, so we skip this
> block as expected for the ignored symlink case.

Sorry, you're quite right; I missed that.

Best wishes,
Chris.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.