[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <46F2B47A.1070902@redhat.com>
Date: Thu, 20 Sep 2007 12:57:14 -0500
From: Eric Sandeen <sandeen@...hat.com>
To: ext4 development <linux-ext4@...r.kernel.org>
Subject: [PATCH e2fsprogs] return status from chattr
This is for RH bug #180596, Chattr command doesn't provide expected
exit code in case of failure.
(trying to clear out an e2fsprogs bug backlog, can you tell?) :)
This is a little funky as a result of the man page saying that
links encountered on recursive traversal are (silently?) ignored.
I changed this a bit so that if it's explicitly listed on the
commandline, the link itself gets chattr'd. I'm not quite sure
what is intended here; that the links are not *followed* or
that they are not chattr'd? Seems a little odd to me.
I tried to follow the way other recursive commands work, for example
chmod -R, and carry on in the face of any errors. If any error
was encountered, exit with an error. If no errors, exit 0.
Also, if both flags and -v (version) are specified, and the flag
set encounters an error, the version set is not attempted. Is this
ok or should both commands be tried?
Finally, I'm curious, the utility ignores anything that's not a link,
regular file, or dir, but the kernel code doesn't have these checks.
Should it?
Comments?
Thanks,
-Eric
Signed-off-by: Eric Sandeen <sandeen@...hat.com>
Index: e2fsprogs-1.40.2/misc/chattr.c
===================================================================
--- e2fsprogs-1.40.2.orig/misc/chattr.c
+++ e2fsprogs-1.40.2/misc/chattr.c
@@ -182,7 +182,7 @@ static int decode_arg (int * i, int argc
static int chattr_dir_proc (const char *, struct dirent *, void *);
-static void change_attributes (const char * name)
+static int change_attributes (const char * name, int cmdline)
{
unsigned long flags;
STRUCT_STAT st;
@@ -190,19 +190,20 @@ static void change_attributes (const cha
if (LSTAT (name, &st) == -1) {
com_err (program_name, errno, _("while trying to stat %s"),
name);
- return;
+ return -1;
}
- if (S_ISLNK(st.st_mode) && recursive)
- return;
- /* Don't try to open device files, fifos etc. We probably
- ought to display an error if the file was explicitly given
- on the command line (whether or not recursive was
- requested). */
- if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode) &&
- !S_ISDIR(st.st_mode))
- return;
+ /* Just silently ignore links found by recursion;
+ not an error according to the manpage */
+ if (S_ISLNK(st.st_mode) && !cmdline)
+ return 0;
+ /* Don't try to open device files, fifos etc. */
+ if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode) &&
+ !S_ISDIR(st.st_mode)) {
+ com_err (program_name, EINVAL, _("for file %s"), name);
+ return -1;
+ }
if (set) {
if (verbose) {
printf (_("Flags of %s set as "), name);
@@ -212,10 +213,11 @@ static void change_attributes (const cha
if (fsetflags (name, sf) == -1)
perror (name);
} else {
- if (fgetflags (name, &flags) == -1)
+ if (fgetflags (name, &flags) == -1) {
com_err (program_name, errno,
_("while reading flags on %s"), name);
- else {
+ return -1;
+ } else {
if (rem)
flags &= ~rf;
if (add)
@@ -227,25 +229,32 @@ static void change_attributes (const cha
}
if (!S_ISDIR(st.st_mode))
flags &= ~EXT2_DIRSYNC_FL;
- if (fsetflags (name, flags) == -1)
+ if (fsetflags (name, flags) == -1) {
com_err (program_name, errno,
_("while setting flags on %s"), name);
+ return -1;
+ }
+
}
}
if (set_version) {
if (verbose)
printf (_("Version of %s set as %lu\n"), name, version);
- if (fsetversion (name, version) == -1)
+ if (fsetversion (name, version) == -1) {
com_err (program_name, errno,
_("while setting version on %s"), name);
+ return -1;
+ }
}
if (S_ISDIR(st.st_mode) && recursive)
- iterate_on_dir (name, chattr_dir_proc, NULL);
+ return iterate_on_dir (name, chattr_dir_proc, NULL);
}
static int chattr_dir_proc (const char * dir_name, struct dirent * de,
void * private EXT2FS_ATTR((unused)))
{
+ int err;
+
if (strcmp (de->d_name, ".") && strcmp (de->d_name, "..")) {
char *path;
@@ -253,11 +262,13 @@ static int chattr_dir_proc (const char *
if (!path) {
fprintf(stderr, _("Couldn't allocate path variable "
"in chattr_dir_proc"));
- exit(1);
+ return -1;
}
sprintf (path, "%s/%s", dir_name, de->d_name);
- change_attributes (path);
+ err = change_attributes (path, 0);
free(path);
+ if (err)
+ return -1;
}
return 0;
}
@@ -266,6 +277,7 @@ int main (int argc, char ** argv)
{
int i, j;
int end_arg = 0;
+ int err, retval = 0;
#ifdef ENABLE_NLS
setlocale(LC_MESSAGES, "");
@@ -303,7 +315,10 @@ int main (int argc, char ** argv)
if (verbose)
fprintf (stderr, "chattr %s (%s)\n",
E2FSPROGS_VERSION, E2FSPROGS_DATE);
- for (j = i; j < argc; j++)
- change_attributes (argv[j]);
- exit(0);
+ for (j = i; j < argc; j++) {
+ err = change_attributes (argv[j], 1);
+ if (err)
+ retval = 1;
+ }
+ exit(retval);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists