Re: error: Support the compiler's control flow analysis better (was: copy-file: Silence gcc warnings)

[Jim Meyering]

On Wed, May 31, 2023 at 9:12 AM Pádraig Brady <P@draigbrady.com> wrote:
> On 30/05/2023 22:29, Paul Eggert wrote:
> > On 5/28/23 06:07, Pádraig Brady wrote:
> >> There still is a gotcha (hit in dd.c in coreutils)
> >> where if you define an error macro yourself
> >> you get a macro redefinition error,
> >
> > I see you fixed that by adding a quick "#define _GL_NO_INLINE_ERROR" to
> > coreutils/src/dd.c. It's a bit cleaner to fix the underlying naming
> > problem instead, so that dd.c need not define the Gnulib internals macro
> > (or its own quirky error macro), so I installed the attached to
> > coreutils to do that.
>
> I was debating that option but decided against it
> as we'd then lose some of the syntax checks on error(args).
> But we can augment the syntax checks to cater for this class of function,
> which I'm doing as follows.
>
> cheers,
> Pádraig
>
> diff --git a/cfg.mk b/cfg.mk
> index 263bc0cfd..64db2bec4 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -189,12 +189,15 @@ sc_prohibit_quotes_notation:
>                 exit 1; }  \
>            || :
>
> +error_fns = (error|die|diagnose)
> +
>   # Files in src/ should quote all strings in error() output, so that
>   # unexpected input chars like \r etc. don't corrupt the error.
>   # In edge cases this can be avoided by putting the format string
>   # on a separate line to the arguments, or the arguments in parenthesis.
>   sc_error_quotes:
> -       @cd $(srcdir)/src && GIT_PAGER= git grep -n 'error *(.*%s.*, 
> [^(]*);$$'\
> +       @cd $(srcdir)/src \
> +         && GIT_PAGER= git grep -E -n '${error_fns} *\(.*%s.*, [^(]*\);$$' \
>            *.c | grep -v ', q' \
>            && { echo '$(ME): '"Use quote() for error string arguments" 1>&2; \
>                 exit 1; }  \
> @@ -206,7 +209,7 @@ sc_error_quotes:
>   sc_error_shell_quotes:
>          @cd $(srcdir)/src && \
>            { GIT_PAGER= git grep -E \
> -           'error \(.*%s[:"], .*(name|file)[^"]*\);$$' *.c; \
> +           '${error_fns} \(.*%s[:"], .*(name|file)[^"]*\);$$' *.c; \

Thanks!
Was there a reason to prefer curly braces there, rather than the more
conventional parentheses?

           '$(error_fns) \(.*%s[:"], .*(name|file)[^"]*\);$$' *.c; \