GNU bug report logs - #35137
[df] incorrect parsing of /proc/self/mountinfo with \r in mount path

Previous Next

Full log

Message #14 received at 35137 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>,
 Zbigniew Jędrzejewski-Szmek <zbyszek <at> in.waw.pl>,
 35137 <at> debbugs.gnu.org, bug-gnulib <bug-gnulib <at> gnu.org>
Subject: Re: bug#35137: [df] incorrect parsing of /proc/self/mountinfo with \r
 in mount path
Date: Tue, 9 Apr 2019 19:15:04 -0700

Bernhard Voelker wrote:

+/* Find the next white space in STR, terminate the string there in place,
+   and return that position.  Otherwise return NULL.  */
+
+static char *
+terminate_at_blank (char const *str)
+{
+  char *s = NULL;
+  if ((s = strchr (str, ' ')) != NULL)
+    *s = '\0';
+  return s;
+}

Since the function modifies its argument, the argument type should be char *, 
not char const *. Also, the code has an assignment in an 'if' conditional and 
the comment is not quite right. Better is:

  /* Find the next space in STR, terminate the string there in place,
     and return that position.  Otherwise return NULL.  */

  static char *
  terminate_at_blank (char *str)
  {
    char *s = strchr (str, ' ');
    if (s)
      *s = '\0';
    return s;
  }

> +            if (! (blank = terminate_at_blank (mntroot)))
> +              continue;

Avoid assignments in 'if' conditionals. Better is:

    blank = terminate_at_blank (target);
    if (! blank)
      continue;

+            if (*source == ' ')
+              {
+                /* The source is an empty string, which is e.g. allowed for
+                   tmpfs: "mount -t tmpfs '' /mnt".  */
+                *source = '\0';
+              }
+            else
+              {
+                if (! (blank = terminate_at_blank (source)))
+                  continue;
+              }

Since 'blank' is not used later, surely these 11 lines of code can be simplified 
to 2 lines:

    if (! terminate_at_blank (source))
      continue;

> +            int mntroot_s;
> +            char *mntroot, *blank, *target, *dash, *fstype, *source;

I suggest using C99-style declaration-after-statement style rather than this 
old-fashioned C89-style declarations-at-start-of-block style, just for the 
changed part of the code anyway.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.