Re: bug#35137: [df] incorrect parsing of /proc/self/mountinfo with \r in mount path

2019-04-10 Thread Zbigniew Jędrzejewski-Szmek
Hi,

I don't know this codebase, so can't comment on the patch, but the
same bug in util-linux was solved by ditching scanf.

https://github.com/karelzak/util-linux/commit/e902609400a861dbdb47d5c3eb98b951530bf01d
https://github.com/karelzak/util-linux/commit/e3782bf6776dcef329b09f4324e1be680f690f3c

Zbyszek


On Tue, Apr 09, 2019 at 07:15:04PM -0700, 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.
> 



Re: [PATCH] Revert part of f406941a8a2ec

2015-10-06 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Oct 06, 2015 at 12:48:45AM -0700, Paul Eggert wrote:
> Thanks, instead of reverting I'd rather fix the bug.  I assume the
> attached patch does that; if not please let me know.

Looks like it should work.

BTW., is it really worth to do anything for the motivation given in
original patch (that editing the file might go wrong under some
old encoding)? Nowadays, anyone should be simply using utf-8 encoding
for their own sake, and for developers there isn't any excuse not to
do that.

Zbyszek

> From 0bb9ebe5c7fed1275a188fe53a23c6504e9eeb13 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <egg...@cs.ucla.edu>
> Date: Tue, 6 Oct 2015 00:46:02 -0700
> Subject: [PATCH] unicase/locale-language: fix typo in utf-8 cookie
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> * lib/unicase/locale-languages.gperf: Fix gperf input file format.
> Problem reported by Zbigniew Jędrzejewski-Szmek.
> ---
>  ChangeLog  | 6 ++
>  lib/unicase/locale-languages.gperf | 3 +--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 28016ca..4f2d6ce 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-10-06  Paul Eggert  <egg...@cs.ucla.edu>
> +
> + unicase/locale-language: fix typo in utf-8 cookie
> + * lib/unicase/locale-languages.gperf: Fix gperf input file format.
> + Problem reported by Zbigniew Jędrzejewski-Szmek.
> +
>  2015-10-02  Paul Eggert  <egg...@cs.ucla.edu>
>  
>   xalloc: do not worry about GCC 5 warning on 32 bit
> diff --git a/lib/unicase/locale-languages.gperf 
> b/lib/unicase/locale-languages.gperf
> index fd566f9..0306576 100644
> --- a/lib/unicase/locale-languages.gperf
> +++ b/lib/unicase/locale-languages.gperf
> @@ -268,10 +268,9 @@
>  "zh", /* Chinese" */
>  "zu", /* Zulu" */
>  "zap", /* Zapotec" */
> -%{
> +%%
>  /*
>   * Local Variables:
>   * coding: utf-8
>   * End:
>   */
> -%}
> -- 
> 2.1.0
> 




[PATCH] Revert part of f406941a8a2ec

2015-10-05 Thread Zbigniew Jędrzejewski-Szmek
Build fails with:
./unicase/locale-languages.gperf:271: declarations are not allowed in the 
keywords section.
To declare a keyword starting with %, enclose it in double-quotes.
(On Fedora 24 rawhide)
---
 lib/unicase/locale-languages.gperf | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/lib/unicase/locale-languages.gperf 
b/lib/unicase/locale-languages.gperf
index fd566f9971..da0d94e7b1 100644
--- a/lib/unicase/locale-languages.gperf
+++ b/lib/unicase/locale-languages.gperf
@@ -268,10 +268,3 @@
 "zh", /* Chinese" */
 "zu", /* Zulu" */
 "zap", /* Zapotec" */
-%{
-/*
- * Local Variables:
- * coding: utf-8
- * End:
- */
-%}
-- 
2.1.0