On 04/29/2011 10:55 AM, Jim Meyering wrote:
>> 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.

For the record, clients like coreutils will not hit the abort() (nor are
they vulnerable to bad memory references pre-patch); this is because
coreutils converts a string from quoting_style_args into a value from
quoting_style_vals before calling any quotearg_*style function, and the
quoting_style_args array specifically excludes an entry for custom
quoting style.

>>
>> * 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.

Here's the form I finally pushed:

diff --git c/lib/quotearg.c w/lib/quotearg.c
index fb49559..da8ba1e 100644
--- c/lib/quotearg.c
+++ w/lib/quotearg.c
@@ -168,10 +168,10 @@ set_custom_quoting (struct quoting_options *o,
 static struct quoting_options
 quoting_options_from_style (enum quoting_style style)
 {
-  struct quoting_options o;
+  struct quoting_options o = { 0 };
+  if (style == custom_quoting_style)
+    abort ();
   o.style = style;
-  o.flags = 0;
-  memset (o.quote_these_too, 0, sizeof o.quote_these_too);
   return o;
 }



-- 
Eric Blake   [email protected]    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to