Bug 92461

Summary: Fix null pointer dereference when CSSParser::sinkFloatingValueList() returns null and is passed to storeVariableDeclaration().
Product: WebKit Reporter: Luke Macpherson <macpherson>
Component: CSSAssignee: Luke Macpherson <macpherson>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, eric, kling, macpherson, menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Luke Macpherson 2012-07-26 22:58:13 PDT
Fix null pointer dereference when CSSParser::sinkFloatingValueList() returns null and is passed to storeVariableDeclaration().
Comment 1 Luke Macpherson 2012-07-26 23:01:32 PDT
Created attachment 154849 [details]
Patch
Comment 2 Luke Macpherson 2012-07-31 17:15:43 PDT
Ping. Could someone please review?
Comment 3 Eric Seidel (no email) 2012-08-03 00:06:15 PDT
Comment on attachment 154849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154849&action=review

> Source/WebCore/css/CSSParser.cpp:3019
> +    if (!value)
> +        return;

A comment here to explain the "why" might be helpful.
Comment 4 Eric Seidel (no email) 2012-08-03 00:08:00 PDT
Comment on attachment 154849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154849&action=review

>> Source/WebCore/css/CSSParser.cpp:3019
>> +        return;
> 
> A comment here to explain the "why" might be helpful.

Something like:
// When CSSGrammar.y encounters an invalid/un-parseable declaration it passes null for the CSSParserValueList, just bail.
Comment 5 Eric Seidel (no email) 2012-08-03 00:08:55 PDT
You don't have to make the comment, but if I were writing the code, I'd be tempted to explain the "why" as null checks are often mysterious as to their necessity or not.  This one you're claiming to be necessary based on how BISON behaves.
Comment 6 Luke Macpherson 2012-08-05 16:54:22 PDT
Created attachment 156573 [details]
Patch for landing
Comment 7 WebKit Review Bot 2012-08-05 18:22:57 PDT
Comment on attachment 156573 [details]
Patch for landing

Clearing flags on attachment: 156573

Committed r124723: <http://trac.webkit.org/changeset/124723>
Comment 8 WebKit Review Bot 2012-08-05 18:23:20 PDT
All reviewed patches have been landed.  Closing bug.