Re: [PATCH] remove nested functions from regcomp.c

[Will Newton <will dot newton at linaro dot org>]

On 29 September 2014 21:54, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> Hi,
>
> Please review the patch that removes nested functions from posix/regcomp.c.
> The patch does not noticeably affect the generated code (same
> instructions, a few differences in used registers, offsets, etc)
> because all the affected functions have __attribute ((always_inline)).
>
> No regressions in 'make check' on x86_64-linux-gnu (Ubuntu 14.04)
>
> This code has some #ifdef branches for "not _LIBC",
> please let me know if any separate testing is required for that case.

This file is shared with gnulib so ideally the changes should be OKed
there too. Unfortunately this particular file seems to have diverged a
bit from gnulib so that might not be that straightforward.

> This change makes GCC emit 4 new warning that all look like this:
> ==================
> regcomp.c: In function âparse_expressionâ:
> regcomp.c:2804:21: warning: âextraâ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
>     name_len == extra[idx]
>                      ^
> regcomp.c:3055:24: note: âextraâ was declared here
>    const unsigned char *extra;
> ==================
> The warnings look incorrect, since the code is doing this:
>   if (nrules)
>        extra = ....
>   ...
>   if (nrules)
>      use(extra)
>
> This feature of GCC is known to have various problems and false
> positives, but the warnings
> did not fire before because nested functions inhibit this warning.
> The warnings can be avoided by initializing all these variables to 0
> at declaration.
>
> 2014-09-29  Kostya Serebryany  <konstantin.s.serebryany@gmail.com>
>
>         * posix/regcomp.c
>         (seek_collating_symbol_entry): New function, broken out of
>         parse_bracket_exp.
>         (lookup_collation_sequence_value): Ditto.
>         (build_range_exp): Ditto.
>         (build_collating_symbol): Ditto.
>         (parse_bracket_exp): Remove nested functions.  Update call sites.
>
>
> --kcc



-- 
Will Newton
Toolchain Working Group, Linaro