lsattr: return error values

PATCH Superseded Eric Sandeen 2 participants 4 messages Jun 18, 2009 – Jun 22, 2009

lore

Share & tools

lore

Eric Sandeen

author

2009-06-18 21:59 UTC #0 lore raw

PATCH 1/1 lsattr: return error values

RH bug 489841 points out that lsattr doesn't return an error if you point it at a file that doesn't exist.

This is slightly trickier because it can take more than one file as an arg, but ls seems to report an error if any occurred, so this does the same, it'll report the last error that was encountered.

Signed-off-by Eric Sandeen <[email protected]>

---

misc/lsattr.c +26 ~~-15~~

index 254ebb4..39fe564 100644
@@ -74,22 +74,25 @@ static void usage(void)
 	exit(1);
 }
 
-static void list_attributes (const char * name)
+static int list_attributes (const char * name)
 {
 	unsigned long flags;
 	unsigned long generation;
+	int retval = 0;
 
 	if (fgetflags (name, &flags) == -1) {
-		com_err (program_name, errno, _("While reading flags on %s"),
+		retval = errno;
+		com_err (program_name, retval, _("While reading flags on %s"),
 			 name);
-		return;
+		return retval;
 	}
 	if (generation_opt) {
 		if (fgetversion (name, &generation) == -1) {
-			com_err (program_name, errno,
+			retval = errno;
+			com_err (program_name, retval,
 				 _("While reading version on %s"),
 				 name);
-			return;
+			return retval;
 		}
 		printf ("%5lu ", generation);
 	}

@@ -101,23 +104,27 @@ static void list_attributes (const char * name)
 		print_flags(stdout, flags, pf_options);
 		printf(" %s\n", name);
 	}
+	return retval;
 }
 
 static int lsattr_dir_proc (const char *, struct dirent *, void *);
 
-static void lsattr_args (const char * name)
+static int lsattr_args (const char * name)
 {
 	STRUCT_STAT	st;
+	int		retval = 0;
 
-	if (LSTAT (name, &st) == -1)
-		com_err (program_name, errno, _("while trying to stat %s"),
+	if (LSTAT (name, &st) == -1) {
+		retval= errno;
+		com_err (program_name, retval, _("while trying to stat %s"),
 			 name);
-	else {
+	} else {
 		if (S_ISDIR(st.st_mode) && !dirs_opt)
-			iterate_on_dir (name, lsattr_dir_proc, NULL);
+			retval = iterate_on_dir (name, lsattr_dir_proc, NULL);
 		else
-			list_attributes (name);
+			retval = list_attributes (name);
 	}
+	return retval;
 }
 
 static int lsattr_dir_proc (const char * dir_name, struct dirent * de,

@@ -155,6 +162,7 @@ int main (int argc, char ** argv)
 {
 	int c;
 	int i;
+	int err, retval = 0;
 
 #ifdef ENABLE_NLS
 	setlocale(LC_MESSAGES, "");

@@ -193,9 +201,12 @@ int main (int argc, char ** argv)
 		fprintf (stderr, "lsattr %s (%s)\n",
 			 E2FSPROGS_VERSION, E2FSPROGS_DATE);
 	if (optind > argc - 1)
-		lsattr_args (".");
+		retval = lsattr_args (".");
 	else
-		for (i = optind; i < argc; i++)
-			lsattr_args (argv[i]);
-	exit(0);
+		for (i = optind; i < argc; i++) {
+			err = lsattr_args (argv[i]);
+			if (err)
+				retval = err;
+		}
+	exit(retval);
 }

Eric Sandeen

author

2009-06-18 22:35 UTC #1 lore raw

Quoted 7 lines · Eric Sandeen

RH bug 489841 points out that lsattr doesn't return an error if you point it at a file that doesn't exist.
This is slightly trickier because it can take more than one file as an arg, but ls seems to report an error if any occurred, so this does the same, it'll report the last error that was encountered.

oops kernel on the brain, I shouldn't be exiting with the errno should I, sigh.

I'll resend :)

-Eric

Quoted 103 lines

Signed-off-by Eric Sandeen <[email protected]>

---

misc/lsattr.c +26 ~~-16~~

index 254ebb4..39fe564 100644
@@ -74,22 +74,25 @@ static void usage(void)
 	exit(1);
 }
 
-static void list_attributes (const char * name)
+static int list_attributes (const char * name)
 {
 	unsigned long flags;
 	unsigned long generation;
+	int retval = 0;
 
 	if (fgetflags (name, &flags) == -1) {
-		com_err (program_name, errno, _("While reading flags on %s"),
+		retval = errno;
+		com_err (program_name, retval, _("While reading flags on %s"),
 			 name);
-		return;
+		return retval;
 	}
 	if (generation_opt) {
 		if (fgetversion (name, &generation) == -1) {
-			com_err (program_name, errno,
+			retval = errno;
+			com_err (program_name, retval,
 				 _("While reading version on %s"),
 				 name);
-			return;
+			return retval;
 		}
 		printf ("%5lu ", generation);
 	}

@@ -101,23 +104,27 @@ static void list_attributes (const char * name)
 		print_flags(stdout, flags, pf_options);
 		printf(" %s\n", name);
 	}
+	return retval;
 }
 
 static int lsattr_dir_proc (const char *, struct dirent *, void *);
 
-static void lsattr_args (const char * name)
+static int lsattr_args (const char * name)
 {
 	STRUCT_STAT	st;
+	int		retval = 0;
 
-	if (LSTAT (name, &st) == -1)
-		com_err (program_name, errno, _("while trying to stat %s"),
+	if (LSTAT (name, &st) == -1) {
+		retval= errno;
+		com_err (program_name, retval, _("while trying to stat %s"),
 			 name);
-	else {
+	} else {
 		if (S_ISDIR(st.st_mode) && !dirs_opt)
-			iterate_on_dir (name, lsattr_dir_proc, NULL);
+			retval = iterate_on_dir (name, lsattr_dir_proc, NULL);
 		else
-			list_attributes (name);
+			retval = list_attributes (name);
 	}
+	return retval;
 }
 
 static int lsattr_dir_proc (const char * dir_name, struct dirent * de,

@@ -155,6 +162,7 @@ int main (int argc, char ** argv)
 {
 	int c;
 	int i;
+	int err, retval = 0;
 
 #ifdef ENABLE_NLS
 	setlocale(LC_MESSAGES, "");

@@ -193,9 +201,12 @@ int main (int argc, char ** argv)
 		fprintf (stderr, "lsattr %s (%s)\n",
 			 E2FSPROGS_VERSION, E2FSPROGS_DATE);
 	if (optind > argc - 1)
-		lsattr_args (".");
+		retval = lsattr_args (".");
 	else
-		for (i = optind; i < argc; i++)
-			lsattr_args (argv[i]);
-	exit(0);
+		for (i = optind; i < argc; i++) {
+			err = lsattr_args (argv[i]);
+			if (err)
+				retval = err;
+		}
+	exit(retval);
 }
 
--

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric Sandeen

author

2009-06-18 22:51 UTC #2 lore raw

PATCH v2 1/1 lsattr: return error values

In reply to Eric Sandeen

(hastily redone before I leave for a few days, hope this is sane, review welcome) :)

lsattr doesn't return an error if you point it at a file that doesn't exist.

This is slightly trickier because it can take more than one file as an arg, but ls seems to report an error if any occurred, so this does the same, it'll report the last error that was encountered.

Addresses-RedHat-Bugzilla: #489841

Signed-off-by Eric Sandeen <[email protected]>

---

misc/lsattr.c +0 -0

index 254ebb4..15b17ad 100644

misc/lsattr.c +24 ~~-14~~

@@ -74,7 +74,7 @@ static void usage(void)
 	exit(1);
 }
 
-static void list_attributes (const char * name)
+static int list_attributes (const char * name)
 {
 	unsigned long flags;
 	unsigned long generation;

@@ -82,14 +82,14 @@ static void list_attributes (const char * name)
 	if (fgetflags (name, &flags) == -1) {
 		com_err (program_name, errno, _("While reading flags on %s"),
 			 name);
-		return;
+		return -1;
 	}
 	if (generation_opt) {
 		if (fgetversion (name, &generation) == -1) {
 			com_err (program_name, errno,
 				 _("While reading version on %s"),
 				 name);
-			return;
+			return -1;
 		}
 		printf ("%5lu ", generation);
 	}

@@ -101,23 +101,27 @@ static void list_attributes (const char * name)
 		print_flags(stdout, flags, pf_options);
 		printf(" %s\n", name);
 	}
+	return 0;
 }
 
 static int lsattr_dir_proc (const char *, struct dirent *, void *);
 
-static void lsattr_args (const char * name)
+static int lsattr_args (const char * name)
 {
 	STRUCT_STAT	st;
+	int retval = 0;
 
-	if (LSTAT (name, &st) == -1)
+	if (LSTAT (name, &st) == -1) {
 		com_err (program_name, errno, _("while trying to stat %s"),
 			 name);
-	else {
+		retval = -1;
+	} else {
 		if (S_ISDIR(st.st_mode) && !dirs_opt)
-			iterate_on_dir (name, lsattr_dir_proc, NULL);
+			retval = iterate_on_dir (name, lsattr_dir_proc, NULL);
 		else
-			list_attributes (name);
+			retval = list_attributes (name);
 	}
+	return retval;
 }
 
 static int lsattr_dir_proc (const char * dir_name, struct dirent * de,

@@ -155,6 +159,7 @@ int main (int argc, char ** argv)
 {
 	int c;
 	int i;
+	int err, retval = 0;
 
 #ifdef ENABLE_NLS
 	setlocale(LC_MESSAGES, "");

@@ -192,10 +197,15 @@ int main (int argc, char ** argv)
 	if (verbose)
 		fprintf (stderr, "lsattr %s (%s)\n",
 			 E2FSPROGS_VERSION, E2FSPROGS_DATE);
-	if (optind > argc - 1)
-		lsattr_args (".");
-	else
-		for (i = optind; i < argc; i++)
-			lsattr_args (argv[i]);
-	exit(0);
+	if (optind > argc - 1) {
+		if (lsattr_args (".") == -1)
+			retval = 1;
+	} else {
+		for (i = optind; i < argc; i++) {
+			err = lsattr_args (argv[i]);
+			if (err)
+				retval = 1;
+		}
+	}
+	exit(retval);
 }

Theodore Tso

Applied

2009-06-22 01:48 UTC #3 lore raw

Quoted 6 lines · Eric Sandeen

(hastily redone before I leave for a few days, hope this is sane, review welcome) :)
lsattr doesn't return an error if you point it at a file that doesn't exist.

Quoted 7 lines

This is slightly trickier because it can take more than one file as an arg, but ls seems to report an error if any occurred, so this does the same, it'll report the last error that was encountered.
Addresses-RedHat-Bugzilla: #489841
Signed-off-by Eric Sandeen <[email protected]>

Thanks, applied.

- Ted