Bug 116340 – WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Hugo Parente Lima]

WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0

[Hugo Parente Lima]

Created attachment 202133 [details]
Patch

[Chris Dumez]

Comment on attachment 202133 [details]
Patch

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

> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157
>      RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame);

Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases.

[Hugo Parente Lima]

(In reply to comment #2)
> (From update of attachment 202133 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review
> 
> > Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157
> >      RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame);
> 
> Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases.

Could be, let me see is g++ is smart enough.

[Hugo Parente Lima]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 202133 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review
> > 
> > > Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157
> > >      RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame);
> > 
> > Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases.
> 
> Could be, let me see is g++ is smart enough.

Hmmm, it isn't, findStorageArea() already does that:

line 237: targetFrame = frame;

[Chris Dumez]

Comment on attachment 202133 [details]
Patch

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

>>>> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157
>>>>      RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame);
>>> 
>>> Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases.
>> 
>> Could be, let me see is g++ is smart enough.
> 
> Hmmm, it isn't, findStorageArea() already does that:
> 
> line 237: targetFrame = frame;

No, it doesn't in the errors cases (when it returns 0). Which is why I assume g++ complains.

[Benjamin Poulain]

Comment on attachment 202133 [details]
Patch

findStorageArea() looks like an awful mess :(
Can you fix that instead?

[Hugo Parente Lima]

(In reply to comment #6)
> (From update of attachment 202133 [details])
> findStorageArea() looks like an awful mess :(
> Can you fix that instead?

Just a mess, awful mess is too much for a little piece of code :-).

Yes, this function could be refactored but the warning probably will be triggered anyway, so I'm start to think that the better way to fix it is to just disable this warning "maybe-uninitialized" due to the amount of false-positives generated by it.

[Hugo Parente Lima]

Created attachment 202475 [details]
Patch

[Chris Dumez]

Comment on attachment 202475 [details]
Patch

Looks fine. r=me.

[WebKit Commit Bot]

Comment on attachment 202475 [details]
Patch

Clearing flags on attachment: 202475

Committed r150513: <http://trac.webkit.org/changeset/150513>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Another way to have fixed this warning would be to have just added this line to the top of the function:

    targetFrame = 0;

We could have left the rest alone.

[Hugo Parente Lima]

(In reply to comment #12)
> Another way to have fixed this warning would be to have just added this line to the top of the function:
> 
>     targetFrame = 0;
> 
> We could have left the rest alone.

OTOH causes a double initialization in normal cases, of a cheap pointer, but causes =]