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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html