Subject: Re: bug#11922: df doesn't handle \n in mount entries appropriately

From: Pádraig Brady <[email-masked]>
Date: Fri, 13 Jul 2012 15:57:54 +0100

On 07/12/2012 05:11 PM, Pádraig Brady wrote:
> On 07/12/2012 04:35 PM, Paul Eggert wrote:
>> On 07/12/2012 08:28 AM, Pádraig Brady wrote:
>>> more problematically it would change the output from
>>> the lib
>>
>> Ah, sorry, I was talking only about df's output,
>> independently of how it gets the info from the underlying
>> library.  Surely the C library API should return a string
>> with raw newlines etc. in it, leaving it up to 'df' how to
>> display it?
>>
> 
> Oops, right.
> I misread "with the lower level" as "in the lower level" :)
> 
> So just considering df, the tradeoff is ambiguous
> output in the presence of \n when just escaping \n,
> or unambiguous output which is backwards incompatible
> in the presence of <space>,<tab>,<newline>.
> 
> It's interesting that the low level interface only
> considers the above 3 chars. I guess that's so
> that _programs_ can use isblank() and getline() to
> parse the output robustly.
> However it might be better for df to escape all
> control characters (\r,\f,...) so that _humans_
> can parse too.

That attached patch takes the simplest approach
and just replaces '\n' with '?'.
Reasons are detailed in the commit message.

I was considering doing something a bit more general,
along the lines of unicode_displayable() in fslint:
http://code.google.com/p/fslint/source/browse/tags/2.42/fslint-gui#92
While that could be generally useful within coreutils,
I've gone with the simpler approach for now.

cheers,
Pádraig.
From 7cd2d023af39262ef134d5cc2d1203432fffb807 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 13 Jul 2012 02:27:26 +0100
Subject: [PATCH] df: don't output '\n' characters in a mount point name

It's awkward to read and problematic for scripts when
new lines not associated with the record delimiters are output.

Note other fields are already handled with mbsalign,
which converts non printable chars to the replacement char.
The caveat with that though is the replacement char takes
a place in the field and so possibly truncates the field
if it was the widest field in the records.

Note other non printable characters like '\r', '\f'
are still output in the mount point field, but at least
these shouldn't be problematic to scripts.

Note <space> and <tab> are unaffected in any field,
which could impact scripts processing the output.
However any of the number fields at least could have
spaces considering `LANG=fr_FR df -B\'1`, so it's
probably best to leave spaces, which also allows
scripts to handle mount points with spaces without change.

* src/df.c (hide_problematic_chars): Replace '\n' with '\?'.
* NEWS: Mention the fix.
---
 NEWS     |    5 +++++
 src/df.c |   24 +++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index b8d3cbb..b4bd7e4 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   date: invalid date '\260'
   [This bug was present in "the beginning".]
 
+  df no longer outputs '\n' characters present in the mount point name.
+  Such characters are replaced with '?' so for example, scripts consuming
+  lines output by df, can work reliably.
+  [This bug was present in "the beginning".]
+
   head --lines=-N (-n-N) now resets the read pointer of a seekable input file.
   This means that "head -n-3" no longer consumes all of its input, and lines
   not output by head may be processed by other programs.  For example, this
diff --git a/src/df.c b/src/df.c
index 7e30d57..f4cf12f 100644
--- a/src/df.c
+++ b/src/df.c
@@ -192,6 +192,23 @@ static struct option const long_options[] =
   {NULL, 0, NULL, 0}
 };
 
+/* Replace problematic chars with '?'.
+   Since only '\n' is currently considered,
+   this should work in all locales.  */
+
+static char*
+hide_problematic_chars (char *cell)
+{
+  char *p = cell;
+  while (*p)
+    {
+      if (*p == '\n')
+        *p = '?';
+      p++;
+    }
+  return cell;
+}
+
 /* Dynamically allocate a row of pointers in TABLE, which
    can then be accessed with standard 2D array notation.  */
 
@@ -315,6 +332,8 @@ get_header (void)
       if (!cell)
         xalloc_die ();
 
+      hide_problematic_chars (cell);
+
       table[nrows-1][field] = cell;
 
       widths[field] = MAX (widths[field], mbswidth (cell, 0));
@@ -661,7 +680,10 @@ get_dev (char const *disk, char const *mount_point,
         }
 
       if (cell)
-        widths[field] = MAX (widths[field], mbswidth (cell, 0));
+        {
+          hide_problematic_chars (cell);
+          widths[field] = MAX (widths[field], mbswidth (cell, 0));
+        }
       table[nrows-1][field] = cell;
     }
 }
-- 
1.7.6.4