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

Previous Next

Full log

View this message in rfc822 format

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

[Message part 1 (text/plain, inline)]

On 4/10/19 4:15 AM, Paul Eggert wrote:
> 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.

Thanks for the review.
Pushed with all these changes at:
https://git.sv.gnu.org/cgit/gnulib.git/commit/?id=eb8278fefa

For coreutils, I'll push the (attached) gnulib update with a NEWS entry soon,
and then work on tests.

Have a nice day,
Berny

[0001-gnulib-update-to-the-latest.patch (text/x-patch, attachment)]

Previous Next

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