Discussion:
Some df(1) cleanup
Michal Mazurek
2016-02-01 16:17:03 UTC
Permalink
"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.c 9 Oct 2015 01:37:06 -0000 1.54
+++ df.c 1 Feb 2016 09:09:34 -0000
@@ -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.c 9 Oct 2015 01:37:06 -0000 1.54
+++ df.c 1 Feb 2016 09:04:57 -0000
@@ -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.c 9 Oct 2015 01:37:06 -0000 1.54
+++ df.c 1 Feb 2016 09:43:38 -0000
@@ -41,7 +41,6 @@
#include <err.h>
#include <errno.h>
#include <fcntl.h>
-#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
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 -0000 1.14
+++ ext2fs_df.c 1 Feb 2016 09:43:38 -0000
@@ -42,11 +42,8 @@
#include <sys/mount.h>
#include <ufs/ext2fs/ext2fs.h>
#include <ufs/ext2fs/ext2fs_dinode.h>
-#include <unistd.h>
-#include <stdlib.h>
+
#include <string.h>
-#include <fcntl.h>
-#include <fstab.h>

int e2fs_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.c 27 Nov 2015 13:49:41 -0000 1.17
+++ ffs_df.c 1 Feb 2016 09:43:38 -0000
@@ -36,13 +36,10 @@

#include <sys/types.h>
#include <sys/mount.h>
-#include <ufs/ufs/dinode.h>
#include <ufs/ffs/fs.h>
+#include <ufs/ufs/dinode.h>

-#include <unistd.h>
-#include <stdlib.h>
#include <string.h>
-#include <fcntl.h>

int ffs_df(int, char *, struct statfs *);
--
Michal Mazurek
Theo Buehler
2016-02-01 16:41:25 UTC
Permalink
Post by Michal Mazurek
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.
Michael McConville
2016-02-01 16:52:35 UTC
Permalink
Post by Theo Buehler
Post by Michal Mazurek
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.
Theo Buehler
2016-02-01 17:23:33 UTC
Permalink
Post by Theo Buehler
Post by Michal Mazurek
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.
Ingo Schwarze
2016-02-05 15:13:44 UTC
Permalink
Hi,
Post by Theo Buehler
Post by Theo Buehler
Post by Michal Mazurek
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

Loading...