Bug 83642

Summary:

[chromium] Signedness issue with code informing v8 of PatternSkia memory use

Product:

WebKit

Reporter:

Peter Kasting <pkasting>

Component:

Platform

Assignee:

Adrienne Walker <enne>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

enne, jamesr, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Using using for std	none

Peter Kasting

Reported 2012-04-10 17:39:43 PDT

http://trac.webkit.org/changeset/109171 added code to inform v8 about memory used in PatternSkia. However, there's a signedness problem with this code: 62>..\platform\graphics\skia\PatternSkia.cpp(50) : warning C4146: unary minus operator applied to unsigned type, result still unsigned It seems like the result of this will be that v8 gets an increasingly distorted picture of memory use as these pattern objects are destroyed. P.S. Is there some way we could build WebKit with warnings=errors?

Attachments
Patch (2.36 KB, patch) 2012-04-10 18:38 PDT, Adrienne Walker	no flags	Details Formatted Diff Diff
Using using for std (2.47 KB, patch) 2012-04-10 18:42 PDT, Adrienne Walker	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

James Robinson

Comment 1 2012-04-10 17:47:37 PDT

(In reply to comment #0) > P.S. Is there some way we could build WebKit with warnings=errors? Yes. Set the compile setting to warning=error, fix all the warnings that show up, and then check that in.

Adrienne Walker

Comment 2 2012-04-10 18:38:41 PDT

Created attachment 136600 [details] Patch

James Robinson

Comment 3 2012-04-10 18:40:41 PDT

Comment on attachment 136600 [details] Patch OK

Adrienne Walker

Comment 4 2012-04-10 18:42:57 PDT

Created attachment 136603 [details] Using using for std

Peter Kasting

Comment 5 2012-04-10 18:45:23 PDT

Comment on attachment 136603 [details] Using using for std Really? We can't just std::-qualify things? I have so much hatred for "using namespace".

James Robinson

Comment 6 2012-04-10 18:49:54 PDT

Comment on attachment 136603 [details] Using using for std Right: http://www.webkit.org/coding/coding-style.html#using-in-cpp

Adrienne Walker

Comment 7 2012-04-10 19:09:01 PDT

(In reply to comment #5) > (From update of attachment 136603 [details]) > Really? We can't just std::-qualify things? > > I have so much hatred for "using namespace". Tell me about it.

Peter Kasting

Comment 8 2012-04-10 21:45:45 PDT

You guys know more than I do, but I read that as forbidding "using std::XYZ" rather than forbidding just std::-qualifying the actual usages (and avoiding "using" altogether).

WebKit Review Bot

Comment 9 2012-04-10 23:29:39 PDT

Comment on attachment 136603 [details] Using using for std Clearing flags on attachment: 136603 Committed r113830: <http://trac.webkit.org/changeset/113830>

WebKit Review Bot

Comment 10 2012-04-10 23:29:43 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.