Bug 50048 - Need to initialize destination variables before calling GL
Summary: Need to initialize destination variables before calling GL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-24 18:14 PST by Gregg Tavares
Modified: 2010-12-02 23:25 PST (History)
2 users (show)

See Also:


Attachments
Patch (16.34 KB, patch)
2010-11-29 15:57 PST, Gregg Tavares
no flags Details | Formatted Diff | Diff
Patch (8.47 KB, patch)
2010-12-02 18:20 PST, Gregg Tavares
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gregg Tavares 2010-11-24 18:14:33 PST
Need to initialize variables before calling GL because if GL has lost the context then those variables will remain uninitialized.

For example

  GLsize len;
  glGetProgramiv(service_id_, GL_INFO_LOG_LENGTH, &len);
  char* dst = new char[len];

In the case above, if GL has lost the const, len will be uninitialized and quite possibly try to allocate a large amount of memory which may easily cause a crash. The solution is to always initialized the variables.

  GLsize len = 0;  <-----
  glGetProgramiv(service_id_, GL_INFO_LOG_LENGTH, &len);
  char* dst = new char[len];
Comment 1 Gregg Tavares 2010-11-24 18:15:15 PST
I have a patch to fix the places in WebKit where this is an issue.
Comment 2 Kenneth Russell 2010-11-26 12:49:55 PST
It would be great if you could submit your patch. Run prepare-ChangeLog, edit the modified ChangeLogs appropriately, and webkit-patch upload to upload it. See http://webkit.org/coding/contributing.html .
Comment 3 Gregg Tavares 2010-11-29 15:57:19 PST
Created attachment 75075 [details]
Patch
Comment 4 Kenneth Russell 2010-11-29 17:11:20 PST
Comment on attachment 75075 [details]
Patch

The code changes look fine, and sorry for the nitpick, but could you please change your editor's settings to not modify existing whitespace, and regenerate the patch? I've been advised by other WebKit reviewers that whitespace changes should not be done in conjunction with code changes. It would be great if you would file a separate bug and submit a patch just for the whitespace cleanups.
Comment 5 Gregg Tavares 2010-12-02 18:20:45 PST
Created attachment 75447 [details]
Patch
Comment 6 David Levin 2010-12-02 20:01:03 PST
Comment on attachment 75447 [details]
Patch

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

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:846
> +    m_maxTextureSize = 0;

I'm surprised this isn't initialized in the constructor.
Comment 7 WebKit Commit Bot 2010-12-02 23:25:24 PST
Comment on attachment 75447 [details]
Patch

Clearing flags on attachment: 75447

Committed r73244: <http://trac.webkit.org/changeset/73244>
Comment 8 WebKit Commit Bot 2010-12-02 23:25:29 PST
All reviewed patches have been landed.  Closing bug.