Re: Some df(1) cleanup
Hi, Theo Buehler wrote on Mon, Feb 01, 2016 at 06:23:33PM +0100: > On Mon, Feb 01, 2016 at 05:41:25PM +0100, Theo Buehler wrote: >> On Mon, Feb 01, 2016 at 05:17:03PM +0100, Michal Mazurek wrote: >>> mntbuf = calloc(argc, sizeof(struct statfs)); >>> if (mntbuf == NULL) >>> - err(1, NULL); >>> + err(1, "calloc"); >> I disagree with the changes in this patch. If either malloc or calloc >> fails it will typically set errno to ENOMEM, so you'll >> >> df: Cannot allocate memory >> >> I don't think adding the information that it was malloc or calloc that >> failed is helpful at all. > Well, to be fair, it's done this way in the malloc manual, Fixed, it was completely obvious that this was bad advice in the manual. The reason why the manual was written like that probably was that the manual focusses on something else (preventing integer overflow and memory leaks) and nobody ever considered style issues of error messages. Yours, Ingo
Re: Some df(1) cleanup
On Mon, Feb 01, 2016 at 05:41:25PM +0100, Theo Buehler wrote: > On Mon, Feb 01, 2016 at 05:17:03PM +0100, Michal Mazurek wrote: > > Meaningful error messages: > > > > mntbuf = calloc(argc, sizeof(struct statfs)); > > if (mntbuf == NULL) > > - err(1, NULL); > > + err(1, "calloc"); > > I disagree with the changes in this patch. If either malloc or calloc > fails it will typically set errno to ENOMEM, so you'll > > df: Cannot allocate memory > > I don't think adding the information that it was malloc or calloc that > failed is helpful at all. Well, to be fair, it's done this way in the malloc manual, but still I'm not in favor of changing this. The header cleanup is ok tb@ if anyone wants to commit that. I can't judge the correctness of the change of the comment. There never was a fsmask variable in df.c since it was imported.
Re: Some df(1) cleanup
Theo Buehler wrote: > On Mon, Feb 01, 2016 at 05:17:03PM +0100, Michal Mazurek wrote: > > Meaningful error messages: > > > > mntbuf = calloc(argc, sizeof(struct statfs)); > > if (mntbuf == NULL) > > - err(1, NULL); > > + err(1, "calloc"); > > I disagree with the changes in this patch. If either malloc or calloc > fails it will typically set errno to ENOMEM, so you'll > > df: Cannot allocate memory > > I don't think adding the information that it was malloc or calloc that > failed is helpful at all. Seconded. IIUC, the only cause of an error other than ENOMEM is a bug in the allocator. This will definitely or almost definitely happen in a part of the allocator that is agnostic to the function called (malloc, calloc, etc.). So printing the function name is just noise.
Re: Some df(1) cleanup
On Mon, Feb 01, 2016 at 05:17:03PM +0100, Michal Mazurek wrote: > Meaningful error messages: > > mntbuf = calloc(argc, sizeof(struct statfs)); > if (mntbuf == NULL) > - err(1, NULL); > + err(1, "calloc"); I disagree with the changes in this patch. If either malloc or calloc fails it will typically set errno to ENOMEM, so you'll df: Cannot allocate memory I don't think adding the information that it was malloc or calloc that failed is helpful at all. I can only speak for myself but I'd appreciate if you would send unrelated patches in separate mails.
Some df(1) cleanup
"fsmask" seems to be a historical variable, but it doesn't exist any
more. I think it was related to -t and -l. Use the word "unselected"
instead of "not in fsmask":
Index: df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.54
diff -u -p -r1.54 df.c
--- df.c9 Oct 2015 01:37:06 - 1.54
+++ df.c1 Feb 2016 09:09:34 -
@@ -249,8 +246,8 @@ maketypelist(char *fslist)
/*
* Make a pass over the filesystem info in ``mntbuf'' filtering out
- * filesystem types not in ``fsmask'' and possibly re-stating to get
- * current (not cached) info. Returns the new count of valid statfs bufs.
+ * unselected filesystem types and possibly re-stating to get current
+ * (not cached) info. Returns the new count of valid statfs bufs.
*/
long
regetmntinfo(struct statfs **mntbufp, long mntsize)
Meaningful error messages:
Index: df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.54
diff -u -p -r1.54 df.c
--- df.c9 Oct 2015 01:37:06 - 1.54
+++ df.c1 Feb 2016 09:04:57 -
@@ -129,7 +129,7 @@ main(int argc, char *argv[])
} else {
mntbuf = calloc(argc, sizeof(struct statfs));
if (mntbuf == NULL)
- err(1, NULL);
+ err(1, "calloc");
mntsize = 0;
for (; *argv; argv++) {
if (stat(*argv, &stbuf) < 0) {
@@ -237,7 +234,7 @@ maketypelist(char *fslist)
/* Build an array of that many types. */
if ((av = typelist = calloc(i + 1, sizeof(char *))) == NULL)
- err(1, NULL);
+ err(1, "calloc");
av[0] = fslist;
for (i = 1, nextcp = fslist; (nextcp = strchr(nextcp, ',')) != NULL;
i++) {
*nextcp = '\0';
Remove uneeded includes, and sort those that can be sorted (some can't):
Index: df.c
===
RCS file: /cvs/src/bin/df/df.c,v
retrieving revision 1.54
diff -u -p -r1.54 df.c
--- df.c9 Oct 2015 01:37:06 - 1.54
+++ df.c1 Feb 2016 09:43:38 -
@@ -41,7 +41,6 @@
#include
#include
#include
-#include
#include
#include
#include
Index: ext2fs_df.c
===
RCS file: /cvs/src/bin/df/ext2fs_df.c,v
retrieving revision 1.14
diff -u -p -r1.14 ext2fs_df.c
--- ext2fs_df.c 27 Nov 2015 13:49:41 - 1.14
+++ ext2fs_df.c 1 Feb 2016 09:43:38 -
@@ -42,11 +42,8 @@
#include
#include
#include
-#include
-#include
+
#include
-#include
-#include
inte2fs_df(int, char *, struct statfs *);
Index: ffs_df.c
===
RCS file: /cvs/src/bin/df/ffs_df.c,v
retrieving revision 1.17
diff -u -p -r1.17 ffs_df.c
--- ffs_df.c27 Nov 2015 13:49:41 - 1.17
+++ ffs_df.c1 Feb 2016 09:43:38 -
@@ -36,13 +36,10 @@
#include
#include
-#include
#include
+#include
-#include
-#include
#include
-#include
intffs_df(int, char *, struct statfs *);
--
Michal Mazurek
