Re: [PATCH] quotearg: avoid uninitialized variable use

[Jim Meyering]

Eric Blake wrote:
> Coverity correctly deduced:
>
> Error: UNINIT:
> m4-1.4.16/lib/quotearg.c:171: var_decl: Declaring variable "o" without 
> initializer.
> m4-1.4.16/lib/quotearg.c:175: uninit_use: Using uninitialized value "o": 
> field "o".right_quote is uninitialized.
>
> When custom_quoting_style was introduced in commit 12247f77,
> this method was not updated, so any caller that passed
> the new enum value to any of the existing quotearg_*style
> functions could trigger a crash from the uninitialized memory.
> That was already documented as unspecified behavior, though,
> so changing to an abort makes it easier to spot bad code that
> passes the wrong enum value, rather than waiting for the
> eventual bad memory dereference later on.
>
> * lib/quotearg.c (quoting_options_from_style): Initialize
> remaining fields, and ensure that custom styles are only used via
> quoting_options rather than quoting_style.

Thanks.  This looks fine.

> Jim, Paul - any objections to this patch?
>
> Hmm, maybe instead of zero-initializing each field, we should
> instead just declare struct quoting_options o = {0}?

I prefer that.

>  ChangeLog      |    5 +++++
>  lib/quotearg.c |    4 ++++
>  2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 34e635b..fb4c7c9 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,10 @@
>  2011-04-28  Eric Blake  <address@hidden>
>
> +     quotearg: avoid uninitialized variable use
> +     * lib/quotearg.c (quoting_options_from_style): Initialize
> +     remaining fields, and ensure that custom styles are only used via
> +     quoting_options rather than quoting_style.
> +
>       getaddrinfo: fix gai_strerror signature
>       * m4/getaddrinfo.m4 (gl_GETADDRINFO): Detect broken signatures,
>       and work around mingw with UNICODE defined.
> diff --git a/lib/quotearg.c b/lib/quotearg.c
> index fb49559..3590cd4 100644
> --- a/lib/quotearg.c
> +++ b/lib/quotearg.c
> @@ -169,9 +169,13 @@ static struct quoting_options
>  quoting_options_from_style (enum quoting_style style)
>  {
>    struct quoting_options o;
> +  if (style = custom_quoting_style)
> +    abort ();
>    o.style = style;
>    o.flags = 0;
>    memset (o.quote_these_too, 0, sizeof o.quote_these_too);
> +  o.left_quote = NULL;
> +  o.right_quote = NULL;
>    return o;
>  }