bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the


From: Jim Meyering
Subject: Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)
Date: Tue, 02 Sep 2008 16:01:13 +0200

Kamil Dudka <address@hidden> wrote:
> On Tuesday 02 September 2008 11:52:57 you wrote:
>> You'll want to print totals with --inodes (-i), too.
> Good point, fixed...
>
>> Please adjust formatting to use spaces before each open parenthesis
> Sorry for this detail, I always forget. Note that this is not acceptable for
> macro definition with arguments (e.g. LOG_EQ).

Of course.  But you can still use a space-after ","
and spaces around operators.  See below.

>> and drop the short-named "-c" option.  There is a strong disincentive
>> to adding new short-named options: they can conflict with other-vendor
>> versions of df.
> Well, dropped...
>
>> It'd be nice to add a test to exercise the code
>> (with and without -i) and at least check for a final line
>> matching /^total.../.
> I wrote two test cases. The 1st is dummy and check only /^total.../ line
> presence and the 2nd computes the summary itself and compare result with
> df --total. The second one requires awk.
>
>> > +#define LOG_EQ(a,b) (((a)&&(b))||(!(a)&&!(b)))
>>
>> This can be written more simply as !((a) ^ (b))
> Replaced with (!(a) == !(b)), thanks James!
>
>> Please split long lines so as not to exceed max length of 80.
> Ok, fixed, new patch in attachment...

Thanks.
Here's an incremental change that fixes some spacing/style
issues as well as:
  - you can depend on AWK being available, now that confnigure relies on it
      so the check.mk change ensures that "make check" passes
      AWK=whatever-autoconf-found to all scripts.
  - re "grep ^total ...", I think such use of `^' is not portable,
      so have quoted those regexp arguments in df/total.

Would you please amend/squash the patch below into your patch and
adjust the line lengths of the log message to be <= 72, so that
the generated ChangeLog lines don't wrap?


diff --git a/src/df.c b/src/df.c
index 1ba7ff3..0bb3b1e 100644
--- a/src/df.c
+++ b/src/df.c
@@ -255,15 +255,15 @@ df_readable (bool negative, uintmax_t n, char *buf,
 }

 /* Logical equivalence */
-#define LOG_EQ(a,b) (!(a)==!(b))
+#define LOG_EQ(a, b) (!(a) == !(b))

 /* Add integral value while using uintmax_t for value part and separate
    negation flag. It adds value of SRC and SRC_NEG to DEST and DEST_NEG.
-   The result will be in DEST and DEST_NEG.  See df_readable () to
-   understand how is the negation flag used. */
+   The result will be in DEST and DEST_NEG.  See df_readable to understand
+   how the negation flag is used.  */
 static void
 add_uint_with_neg_flag (uintmax_t *dest, bool *dest_neg,
-               uintmax_t src, bool src_neg)
+                       uintmax_t src, bool src_neg)
 {
   if (LOG_EQ (*dest_neg, src_neg))
     {
@@ -424,9 +424,9 @@ show_dev (char const *disk, char const *mount_point,

       grand_fsu.fsu_blocks += input_units * total;
       grand_fsu.fsu_bfree  += input_units * available_to_root;
-      add_uint_with_neg_flag (
-         &grand_fsu.fsu_bavail, &grand_fsu.fsu_bavail_top_bit_set,
-         input_units * available, negate_available);
+      add_uint_with_neg_flag (&grand_fsu.fsu_bavail,
+                             &grand_fsu.fsu_bavail_top_bit_set,
+                             input_units * available, negate_available);
     }

   used = UINTMAX_MAX;
diff --git a/tests/check.mk b/tests/check.mk
index 4fca283..03b89dc 100644
--- a/tests/check.mk
+++ b/tests/check.mk
@@ -79,6 +79,7 @@ TESTS_ENVIRONMENT =                           \
   top_srcdir='$(top_srcdir)'                   \
   CONFIG_HEADER='$(abs_top_builddir)/lib/config.h' \
   CU_TEST_NAME=`basename '$(abs_srcdir)'`,$$tst        \
+  AWK='$(AWK)'                                 \
   EGREP='$(EGREP)'                             \
   EXEEXT='$(EXEEXT)'                           \
   MAKE=$(MAKE)                                 \
diff --git a/tests/df/total b/tests/df/total
index be4bc19..5398deb 100755
--- a/tests/df/total
+++ b/tests/df/total
@@ -29,12 +29,12 @@ fail=0
 umask 22

 df > tmp || fail=1
-grep ^total tmp && fail=1
+grep '^total' tmp && fail=1

 df -i > tmp || fail=1
-grep ^total tmp && fail=1
+grep '^total' tmp && fail=1

-df --total | grep ^total || fail=1
-df -i --total | grep ^total || fail=1
+df --total | grep '^total' || fail=1
+df -i --total | grep '^total' || fail=1

 (exit $fail); exit $fail
diff --git a/tests/df/total-awk b/tests/df/total-awk
index e98b8ac..183521c 100755
--- a/tests/df/total-awk
+++ b/tests/df/total-awk
@@ -23,9 +23,6 @@ fi

 . $srcdir/test-lib.sh

-(awk --help) 2>&1 |grep 'Usage: awk' > /dev/null \
-  || skip_test_ "awk not found"
-
 fail=0

 # Don't let a different umask perturb the results.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]