Bug 105679 – [Blackberry] Static code analysis warning fixes

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[sfa]

Klocwork issues: 959,1855, 4084

[sfa]

Created attachment 180607 [details]
patch for klocwork issues: 959, 1855

[sfa]

klocwork issue 4084 withdrawn.

[sfa]

patch has passed EWS, needs review still.

[Rob Buis]

Comment on attachment 180607 [details]
patch for klocwork issues: 959, 1855

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

Looks good, please fix the style nit before landing.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:171
> +        if (page) {

It is Webkit style to combine above two lines.

[Rob Buis]

Comment on attachment 180607 [details]
patch for klocwork issues: 959, 1855

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

Looks good, please fix the style nit before landing.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:171
> +        if (page) {

It is Webkit style to combine above two lines.

[sfa]

There are two accesses to the var 'page' in the if-stmt so combining the lines wouldn't have the same semantics.

The other access is the line further down:

float scaleFactorFudge = 1 / page->deviceScaleFactor();

[Rob Buis]

(In reply to comment #6)
> There are two accesses to the var 'page' in the if-stmt so combining the lines wouldn't have the same semantics.
> 
> The other access is the line further down:
> 
> float scaleFactorFudge = 1 / page->deviceScaleFactor();

I don't think that matters, page is visible in the entire scope of the if block. Can you recheck?

[sfa]

Created attachment 181177 [details]
updated patch

Sorry, I misunderstood which lines you wanted merged together, this patch moves the initialization of the variable into the if-expr.

[Rob Buis]

Comment on attachment 181177 [details]
updated patch

LGTM

[WebKit Review Bot]

Comment on attachment 181177 [details]
updated patch

Rejecting attachment 181177 [details] from review queue.

[e-mail] does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.

[Eric Seidel (no email)]

Comment on attachment 180607 [details]
patch for klocwork issues: 959, 1855

Cleared Rob Buis's review+ from obsolete attachment 180607 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[WebKit Review Bot]

Comment on attachment 181177 [details]
updated patch

Clearing flags on attachment: 181177

Committed r138804: <http://trac.webkit.org/changeset/138804>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.