Thanks for reporting the bug. I installed the attached patch to fix it.
>From af4711d5dc15f0c014bfd9c92f4eadedb89f732f Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sun, 15 Aug 2021 21:29:38 -0700
Subject: [PATCH] chmod: fix use of uninitialized var if -v
Problem reported by Michael Debertol (Bug#50070).
* NEWS: Mention the fix.
* src/chmod.c (struct change_status): New struct, replacing the
old enum Change_status. All uses changed.
(describe_change): Distinguish between cases depending on
whether 'stat' or its equivalent succeeded. Report a line
of output even if 'stat' failed, as that matches the documentation.
Rework to avoid casts.
(process_file): Do not output nonsense modes computed from
uninitialized storage, removing a couple of IF_LINTs. Simplify by
defaulting to CH_NO_STAT.
---
NEWS | 3 ++
src/chmod.c | 127 ++++++++++++++++++++++++++--------------------------
2 files changed, 67 insertions(+), 63 deletions(-)
diff --git a/NEWS b/NEWS
index e14de1397..ddec56bdf 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS -*- outline -*-
** Bug fixes
+ chmod -v no longer misreports modes of dangling symlinks.
+ [bug introduced in coreutils-5.3.0]
+
cp -a --attributes-only now never removes destination files,
even if the destination files are hardlinked, or the source
is a non regular file.
diff --git a/src/chmod.c b/src/chmod.c
index 160a0c537..4447e784e 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -39,12 +39,19 @@
proper_name ("David MacKenzie"), \
proper_name ("Jim Meyering")
-enum Change_status
+struct change_status
{
- CH_NOT_APPLIED,
- CH_SUCCEEDED,
- CH_FAILED,
- CH_NO_CHANGE_REQUESTED
+ enum
+ {
+ CH_NO_STAT,
+ CH_NOT_APPLIED,
+ CH_FAILED,
+ CH_NO_CHANGE_REQUESTED,
+ CH_SUCCEEDED
+ }
+ status;
+ mode_t old_mode;
+ mode_t new_mode;
};
enum Verbosity
@@ -136,30 +143,42 @@ mode_changed (int dir_fd, char const *file, char const *file_full_name,
}
/* Tell the user how/if the MODE of FILE has been changed.
- CHANGED describes what (if anything) has happened. */
+ CH describes what (if anything) has happened. */
static void
-describe_change (char const *file, mode_t old_mode, mode_t mode,
- enum Change_status changed)
+describe_change (char const *file, struct change_status const *ch)
{
char perms[12]; /* "-rwxrwxrwx" ls-style modes. */
char old_perms[12];
char const *fmt;
+ char const *quoted_file = quoteaf (file);
- if (changed == CH_NOT_APPLIED)
+ switch (ch->status)
{
+ case CH_NOT_APPLIED:
printf (_("neither symbolic link %s nor referent has been changed\n"),
- quoteaf (file));
+ quoted_file);
return;
- }
- strmode (mode, perms);
+ case CH_NO_STAT:
+ printf (_("%s could not be accessed\n"), quoted_file);
+ return;
+
+ default:
+ break;
+ }
+
+ unsigned long int
+ old_m = ch->old_mode & CHMOD_MODE_BITS,
+ m = ch->new_mode & CHMOD_MODE_BITS;
+
+ strmode (ch->new_mode, perms);
perms[10] = '\0'; /* Remove trailing space. */
- strmode (old_mode, old_perms);
+ strmode (ch->old_mode, old_perms);
old_perms[10] = '\0'; /* Remove trailing space. */
- switch (changed)
+ switch (ch->status)
{
case CH_SUCCEEDED:
fmt = _("mode of %s changed from %04lo (%s) to %04lo (%s)\n");
@@ -169,15 +188,12 @@ describe_change (char const *file, mode_t old_mode, mode_t mode,
break;
case CH_NO_CHANGE_REQUESTED:
fmt = _("mode of %s retained as %04lo (%s)\n");
- printf (fmt, quoteaf (file),
- (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]);
+ printf (fmt, quoted_file, m, &perms[1]);
return;
default:
abort ();
}
- printf (fmt, quoteaf (file),
- (unsigned long int) (old_mode & CHMOD_MODE_BITS), &old_perms[1],
- (unsigned long int) (mode & CHMOD_MODE_BITS), &perms[1]);
+ printf (fmt, quoted_file, old_m, &old_perms[1], m, &perms[1]);
}
/* Change the mode of FILE.
@@ -190,10 +206,8 @@ process_file (FTS *fts, FTSENT *ent)
char const *file_full_name = ent->fts_path;
char const *file = ent->fts_accpath;
const struct stat *file_stats = ent->fts_statp;
- mode_t old_mode IF_LINT ( = 0);
- mode_t new_mode IF_LINT ( = 0);
- bool ok = true;
- bool chmod_succeeded = false;
+ struct change_status ch;
+ ch.status = CH_NO_STAT;
switch (ent->fts_info)
{
@@ -217,27 +231,23 @@ process_file (FTS *fts, FTSENT *ent)
if (! force_silent)
error (0, ent->fts_errno, _("cannot access %s"),
quoteaf (file_full_name));
- ok = false;
break;
case FTS_ERR:
if (! force_silent)
error (0, ent->fts_errno, "%s", quotef (file_full_name));
- ok = false;
break;
case FTS_DNR:
if (! force_silent)
error (0, ent->fts_errno, _("cannot read directory %s"),
quoteaf (file_full_name));
- ok = false;
break;
case FTS_SLNONE:
if (! force_silent)
error (0, 0, _("cannot operate on dangling symlink %s"),
quoteaf (file_full_name));
- ok = false;
break;
case FTS_DC: /* directory that causes cycles */
@@ -246,13 +256,14 @@ process_file (FTS *fts, FTSENT *ent)
emit_cycle_warning (file_full_name);
return false;
}
- break;
-
+ FALLTHROUGH;
default:
+ ch.status = CH_NOT_APPLIED;
break;
}
- if (ok && ROOT_DEV_INO_CHECK (root_dev_ino, file_stats))
+ if (ch.status == CH_NOT_APPLIED
+ && ROOT_DEV_INO_CHECK (root_dev_ino, file_stats))
{
ROOT_DEV_INO_WARN (file_full_name);
/* Tell fts not to traverse into this hierarchy. */
@@ -262,66 +273,56 @@ process_file (FTS *fts, FTSENT *ent)
return false;
}
- if (ok)
+ if (ch.status == CH_NOT_APPLIED && ! S_ISLNK (file_stats->st_mode))
{
- old_mode = file_stats->st_mode;
- new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value,
- change, NULL);
-
- if (! S_ISLNK (old_mode))
+ ch.old_mode = file_stats->st_mode;
+ ch.new_mode = mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0,
+ umask_value, change, NULL);
+ if (chmodat (fts->fts_cwd_fd, file, ch.new_mode) == 0)
+ ch.status = CH_SUCCEEDED;
+ else
{
- if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
- chmod_succeeded = true;
- else
- {
- if (! force_silent)
- error (0, errno, _("changing permissions of %s"),
- quoteaf (file_full_name));
- ok = false;
- }
+ if (! force_silent)
+ error (0, errno, _("changing permissions of %s"),
+ quoteaf (file_full_name));
+ ch.status = CH_FAILED;
}
}
if (verbosity != V_off)
{
- bool changed = (chmod_succeeded
- && mode_changed (fts->fts_cwd_fd, file, file_full_name,
- old_mode, new_mode));
+ if (ch.status == CH_SUCCEEDED
+ && !mode_changed (fts->fts_cwd_fd, file, file_full_name,
+ ch.old_mode, ch.new_mode))
+ ch.status = CH_NO_CHANGE_REQUESTED;
- if (changed || verbosity == V_high)
- {
- enum Change_status ch_status =
- (!ok ? CH_FAILED
- : !chmod_succeeded ? CH_NOT_APPLIED
- : !changed ? CH_NO_CHANGE_REQUESTED
- : CH_SUCCEEDED);
- describe_change (file_full_name, old_mode, new_mode, ch_status);
- }
+ if (ch.status == CH_SUCCEEDED || verbosity == V_high)
+ describe_change (file_full_name, &ch);
}
- if (chmod_succeeded && diagnose_surprises)
+ if (CH_NO_CHANGE_REQUESTED <= ch.status && diagnose_surprises)
{
mode_t naively_expected_mode =
- mode_adjust (old_mode, S_ISDIR (old_mode) != 0, 0, change, NULL);
- if (new_mode & ~naively_expected_mode)
+ mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, 0, change, NULL);
+ if (ch.new_mode & ~naively_expected_mode)
{
char new_perms[12];
char naively_expected_perms[12];
- strmode (new_mode, new_perms);
+ strmode (ch.new_mode, new_perms);
strmode (naively_expected_mode, naively_expected_perms);
new_perms[10] = naively_expected_perms[10] = '\0';
error (0, 0,
_("%s: new permissions are %s, not %s"),
quotef (file_full_name),
new_perms + 1, naively_expected_perms + 1);
- ok = false;
+ ch.status = CH_FAILED;
}
}
if ( ! recurse)
fts_set (fts, ent, FTS_SKIP);
- return ok;
+ return CH_NO_CHANGE_REQUESTED <= ch.status;
}
/* Recursively change the modes of the specified FILES (the last entry
--
2.30.2
