On 09/30/2012 05:43 PM, Bernhard Voelker wrote:
On 09/29/2012 11:29 PM, Bernhard Voelker wrote:
Here you are (see attached):
[PATCH 20/20] df: print "total" in grand total's target field if no source
[PATCH 19/20] df: cleanup comments regarding double blank after
[PATCH 18/20] df: add a test for the --output option
[PATCH 17/20] df: document the new --output option
[PATCH 16/20] df: plug two minor memory holes detected by valgrind
[PATCH 15/20] df: add new --output option
[PATCH 14/20] df: enhance comment describing history
[PATCH 13/20] df: give posix_format variable a better scope
[PATCH 12/20] df: cleanup header_mode handling regarding --inodes
[PATCH 11/20] df: minor cleanup to improve readability
[PATCH 10/20] df: remove remainder of pre-mixed fields support
[PATCH 09/20] df: add basic support for mixed block and inode fields
[PATCH 08/20] df: print '-' into the target column of the total line
[PATCH 07/20] df: only sum up grand total if required
[PATCH 06/20] df: remove now-obsolescent condition
[PATCH 05/20] df: use enum value for long option --total
[PATCH 04/20] df: apply ambsalign to the last field with
[PATCH 03/20] df: rework internal processing of output columns
[PATCH 02/20] df: rename some displayable fields
[PATCH 01/20] df: move the call of get_header from get_dev to main
The bigger/important changes are in the patches 03, 09, and 15.
Example:
$ src/df -h --tot --o=target,pcent,ipcent / /home
Mounted on Use% IUse%
/ 65% 32%
/home 65% 2%
total 65% 5%
Have a nice day,
Berny
I have added another patch at the end:
[PATCH 21/21] df: ensure that the mount list contains a valid
fstype with --output
The function read_file_system_list only ensures a valid file
system type if the calling parameter is true. With the new
option --output, there is an additional condition needed:
when the FSTYPE_FIELD is used.
I didn't notice a problem on GNU/Linux, but the documentation of
read_file_system_list() leads to the conclusion that df needs to
pass true also for 'df --o=fstype,...'.
The new, additional condition may also have replaced the one before
|| print_type
because
|| field_data[FSTYPE_FIELD].used
also leads to true with 'df -T'; I just left it there for now.
Have a nice day,
Berny
I've finished my review. Sorry for the delay.
I've made a couple of adjustments in the attached df--output-adjustments.patch
The first is to rename --output=total to --output=size
I think having `df --total --output=total` is a bit confusing?
Also I've changed df --output to show the block size in the "size" header,
unless -h (human) is being used. I.E. behave with this header
more like the traditional df output. I think this is needed
as important info is lost about the units otherwise?
I've put the full rebased 24 item patch set here:
http://www.pixelbeat.org/patches/coreutils/df--output.patch.bz2
Before merging I'll probably reorder/squash to 3 patches;
logic, docs and test
thanks,
Pádraig.
>From d34ae68693ebdb1b049479ad50c3ce2c8b823aa7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Wed, 7 Nov 2012 00:45:07 +0000
Subject: [PATCH 1/3] df: change --output=total to --output=size
To avoid confusion with --total
* src/df.c: Rename the argument.
* doc/coreutils.texi (df invocation): Likewise.
* test/df/df-output.sh: Likewise.
---
doc/coreutils.texi | 4 ++--
src/df.c | 6 +++---
tests/df/df-output.sh | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8027c78..b251b3d 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10702,14 +10702,14 @@ The source of the mount point, usually a device.
@item fstype
File system type.
-@item total
+@item size
Total number of blocks.
@item used
Number of used blocks.
@item avail
Number of available blocks.
@item pcent
-Percentage of @var{used} divided by @var{total}.
+Percentage of @var{used} divided by @var{size}.
@item itotal
Total number of inodes.
diff --git a/src/df.c b/src/df.c
index 079fa02..bdeaf03 100644
--- a/src/df.c
+++ b/src/df.c
@@ -166,7 +166,7 @@ static struct field_data_t field_data[] = {
"fstype", OTHER_FLD, N_("Type"), 4, MBS_ALIGN_LEFT, false },
[TOTAL_FIELD] = { TOTAL_FIELD,
- "total", BLOCK_FLD, N_("blocks"), 5, MBS_ALIGN_RIGHT, false },
+ "size", BLOCK_FLD, N_("blocks"), 5, MBS_ALIGN_RIGHT, false },
[USED_FIELD] = { USED_FIELD,
"used", BLOCK_FLD, N_("Used"), 5, MBS_ALIGN_RIGHT, false },
@@ -193,7 +193,7 @@ static struct field_data_t field_data[] = {
"target", OTHER_FLD, N_("Mounted on"), 0, MBS_ALIGN_LEFT, false }
};
-static char const *all_args_string = "source,fstype,total,used,avail,pcent,"
+static char const *all_args_string = "source,fstype,size,used,avail,pcent,"
"itotal,iused,iavail,ipcent,target";
/* Storage for the definition of output columns. */
@@ -1158,7 +1158,7 @@ Mandatory arguments to long options are mandatory for short options too.\n\
emit_size_note ();
fputs (_("\n\
FIELD_LIST is a comma-separated list of columns to be included. Valid\n\
-field names are: 'source', 'fstype', 'total', 'used', 'avail', 'pcent',\n\
+field names are: 'source', 'fstype', 'size', 'used', 'avail', 'pcent',\n\
'itotal', 'iused', 'iavail', 'ipcent' and 'target' (see info page).\n\
"), stdout);
emit_ancillary_info ();
diff --git a/tests/df/df-output.sh b/tests/df/df-output.sh
index 210cce3..34c4049 100644
--- a/tests/df/df-output.sh
+++ b/tests/df/df-output.sh
@@ -70,7 +70,7 @@ cat <<\EOF > exp || framework_failure_
Filesystem Type Size Used Avail Use% Inodes IUsed IFree IUse% Mounted on
EOF
-df --o=source,fstype,total,used,avail,pcent \
+df --o=source,fstype,size,used,avail,pcent \
--o=itotal,iused,iavail,ipcent,target '.' >out || fail=1
sed -e '1 {s/ [ ]*/ /g;q}' out > out2
compare exp out2 || fail=1
--
1.7.6.4
>From c8ae726034d22b7fc8015134587c6dd6df6222ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Wed, 7 Nov 2012 00:52:44 +0000
Subject: [PATCH 2/3] df: with --output but without -h, show the block size
* src/df.c (get_header): Indicate the block size used,
in the "size" header, when using --output without -h.
* tests/df/df-output.sh: Adjust for, and add an extra test for,
the new behavior.
---
src/df.c | 8 +++++++-
tests/df/df-output.sh | 14 ++++++++++++--
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/df.c b/src/df.c
index bdeaf03..751740c 100644
--- a/src/df.c
+++ b/src/df.c
@@ -493,7 +493,10 @@ get_header (void)
char *cell = NULL;
char const *header = _(columns[col]->caption);
- if (header_mode == DEFAULT_MODE && columns[col]->field == TOTAL_FIELD)
+ if (columns[col]->field == TOTAL_FIELD
+ && (header_mode == DEFAULT_MODE
+ || (header_mode == OUTPUT_MODE
+ && !(human_output_opts & human_autoscale))))
{
char buf[LONGEST_HUMAN_READABLE + 1];
@@ -526,6 +529,9 @@ get_header (void)
char *num = human_readable (output_block_size, buf, opts, 1, 1);
+ /* Reset the header back to the default in OUTPUT_MODE. */
+ header = N_("blocks");
+
/* TRANSLATORS: this is the "1K-blocks" header in "df" output. */
if (asprintf (&cell, _("%s-%s"), num, header) == -1)
cell = NULL;
diff --git a/tests/df/df-output.sh b/tests/df/df-output.sh
index 34c4049..0b40b28 100644
--- a/tests/df/df-output.sh
+++ b/tests/df/df-output.sh
@@ -70,12 +70,22 @@ cat <<\EOF > exp || framework_failure_
Filesystem Type Size Used Avail Use% Inodes IUsed IFree IUse% Mounted on
EOF
-df --o=source,fstype,size,used,avail,pcent \
+df -h --o=source,fstype,size,used,avail,pcent \
--o=itotal,iused,iavail,ipcent,target '.' >out || fail=1
sed -e '1 {s/ [ ]*/ /g;q}' out > out2
compare exp out2 || fail=1
-df --output '.' >out || fail=1
+df -h --output '.' >out || fail=1
+sed -e '1 {s/ [ ]*/ /g;q}' out > out2
+compare exp out2 || fail=1
+
+# Ensure that --output indicates the block size
+# when not using --human-readable
+cat <<\EOF > exp || framework_failure_
+1K-blocks
+EOF
+
+df -B1K --output=size '.' >out || fail=1
sed -e '1 {s/ [ ]*/ /g;q}' out > out2
compare exp out2 || fail=1
--
1.7.6.4
>From a1d60dee50a7d75065e35e6ba74ab7412055a3ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Wed, 7 Nov 2012 02:35:51 +0000
Subject: [PATCH 3/3] maint: df: whitespace cleanups
Minor tweaks for squashing.
---
src/df.c | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/df.c b/src/df.c
index 751740c..7f418c4 100644
--- a/src/df.c
+++ b/src/df.c
@@ -279,7 +279,7 @@ alloc_table_row (void)
{
nrows++;
table = xnrealloc (table, nrows, sizeof (char *));
- table[nrows-1] = xnmalloc (ncolumns, sizeof (char *));
+ table[nrows - 1] = xnmalloc (ncolumns, sizeof (char *));
}
/* Output each cell in the table, accounting for the
@@ -290,7 +290,7 @@ print_table (void)
{
size_t row;
- for (row = 0; row < nrows; row ++)
+ for (row = 0; row < nrows; row++)
{
size_t col;
for (col = 0; col < ncolumns; col++)
@@ -331,9 +331,9 @@ alloc_field (int f, const char *c)
{
ncolumns++;
columns = xnrealloc (columns, ncolumns, sizeof (struct field_data_t *));
- columns[ncolumns-1] = &field_data[f];
+ columns[ncolumns - 1] = &field_data[f];
if (c != NULL)
- columns[ncolumns-1]->caption = c;
+ columns[ncolumns - 1]->caption = c;
if (field_data[f].used)
assert (!"field used");
@@ -363,7 +363,7 @@ decode_output_arg (char const *arg)
display_field_t field = -1;
for (unsigned int i = 0; i < ARRAY_CARDINALITY (field_data); i++)
{
- if (STREQ (field_data[i].arg , s))
+ if (STREQ (field_data[i].arg, s))
{
field = i;
break;
@@ -379,7 +379,7 @@ decode_output_arg (char const *arg)
{
/* Prevent the fields from being used more than once. */
error (0, 0, _("option --output: field '%s' used more than once"),
- field_data[field].arg);
+ field_data[field].arg);
usage (EXIT_FAILURE);
}
@@ -420,7 +420,7 @@ static void
get_field_list (void)
{
switch (header_mode)
- {
+ {
case DEFAULT_MODE:
alloc_field (SOURCE_FIELD, NULL);
if (print_type)
@@ -468,15 +468,15 @@ get_field_list (void)
case OUTPUT_MODE:
if (!ncolumns)
- {
- /* Add all fields if --output was given without a field list. */
- decode_output_arg (all_args_string);
- }
+ {
+ /* Add all fields if --output was given without a field list. */
+ decode_output_arg (all_args_string);
+ }
break;
default:
assert (!"invalid header_mode");
- }
+ }
}
/* Obtain the appropriate header entries. */
@@ -553,7 +553,7 @@ get_header (void)
hide_problematic_chars (cell);
- table[nrows-1][col] = cell;
+ table[nrows - 1][col] = cell;
columns[col]->width = MAX (columns[col]->width, mbswidth (cell, 0));
}
@@ -719,7 +719,7 @@ add_to_grand_total (struct field_values_t *bv, struct field_values_t *iv)
if (known_value (bv->total))
grand_fsu.fsu_blocks += bv->input_units * bv->total;
if (known_value (bv->available_to_root))
- grand_fsu.fsu_bfree += bv->input_units * bv->available_to_root;
+ grand_fsu.fsu_bfree += bv->input_units * bv->available_to_root;
if (known_value (bv->available))
add_uint_with_neg_flag (&grand_fsu.fsu_bavail,
&grand_fsu.fsu_bavail_top_bit_set,
@@ -874,7 +874,7 @@ get_dev (char const *disk, char const *mount_point,
&& v->used <= TYPE_MAXIMUM (uintmax_t) / 100
&& v->used + v->available != 0
&& (v->used + v->available < v->used)
- == v->negate_available)
+ == v->negate_available)
{
uintmax_t u100 = v->used * 100;
uintmax_t nonroot_total = v->used + v->available;
@@ -939,7 +939,7 @@ get_dev (char const *disk, char const *mount_point,
hide_problematic_chars (cell);
columns[col]->width = MAX (columns[col]->width, mbswidth (cell, 0));
- table[nrows-1][col] = cell;
+ table[nrows - 1][col] = cell;
}
free (dev_name);
}
@@ -1162,7 +1162,7 @@ Mandatory arguments to long options are mandatory for short options too.\n\
fputs (VERSION_OPTION_DESCRIPTION, stdout);
emit_blocksize_note ("DF");
emit_size_note ();
- fputs (_("\n\
+ fputs (_("\n\
FIELD_LIST is a comma-separated list of columns to be included. Valid\n\
field names are: 'source', 'fstype', 'size', 'used', 'avail', 'pcent',\n\
'itotal', 'iused', 'iavail', 'ipcent' and 'target' (see info page).\n\
@@ -1456,7 +1456,7 @@ main (int argc, char **argv)
error (EXIT_FAILURE, 0, _("no file systems processed"));
}
- IF_LINT ( free (columns));
+ IF_LINT (free (columns));
exit (exit_status);
}
--
1.7.6.4
