Closed
Bug 1479950
Opened 7 years ago
Closed 7 years ago
Incorrect use of "&&" in place of "&" : mozilla/gfx/skia/skia/src/gpu/GrColorSpaceXform.cpp:184:31: warning: enum constant in boolean context [-Wint-in-bool-context] if (SkToBool(a->fFlags && kApplyGamutXform_Flag) && a->fGamutXform != b->fGamutXform
Categories
(Core :: Graphics, defect, P3)
Core
Graphics
Tracking
()
RESOLVED
FIXED
People
(Reporter: ishikawa, Assigned: ishikawa)
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file, 1 obsolete file)
1.05 KB,
patch
|
lsalzman
:
review+
|
Details | Diff | Splinter Review |
Hi,
I am using GCC v7 for local linux build with rather strict warnings/error setting.
I noticed the following warning while I was compiling a source I fetched in the last coupule of days.
/NREF-COMM-CENTRAL/mozilla/gfx/skia/skia/src/gpu/GrColorSpaceXform.cpp:184:31: warning: enum constant in boolean context [-Wint-in-bool-context]
if (SkToBool(a->fFlags && kApplyGamutXform_Flag) && a->fGamutXform != b->fGamutXform) {
^
/NREF-COMM-CENTRAL/mozilla/gfx/skia/skia/include/core/SkTypes.h:203:27: note: in definition of macro ‘SkToBool’
#define SkToBool(cond) ((cond) != 0)
^~~~
Upon inspection, I found the use of "&" is very popular in the file against "fFlags" and no other use of "&&" was seen. So I conclude that the use of |&&| is indeed incorrect and |&| should be used.
The patch follows.
Assignee | ||
Comment 1•7 years ago
|
||
I take that this is summer vacation season.
I picked up the less loaded reviewer for review request, but whoever can take a look and approve it, and furthermore take it over and apply it will be appreciated.
Thank you in advance.
Assignee: nobody → ishikawa
Attachment #8996494 -
Flags: review?(lsalzman)
Comment 2•7 years ago
|
||
Comment on attachment 8996494 [details] [diff] [review]
Finxing the incorrect use of "&&".
This does not actually fix the line mentioned in the warning (line 184).
The line that is being changed in this patch does not even have an && in it, so I am not actually sure how this patch was generated.
Attachment #8996494 -
Flags: review?(lsalzman) → review-
Assignee | ||
Comment 3•7 years ago
|
||
Sorry, I loaded an incorrect patch that was created during source tree shuffle.
This one addresses the correct bug and fixes it.
TIA
Attachment #8996494 -
Attachment is obsolete: true
Attachment #8996591 -
Flags: review?(lsalzman)
Updated•7 years ago
|
Attachment #8996591 -
Flags: review?(lsalzman) → review+
Assignee | ||
Comment 4•7 years ago
|
||
Thank you for the review.
Now, how do I proceed?
It has been close to a year and more since I submitted a patch for inclusion last time.
Today there seem to be newer tools aside from bugzilla for patch submission/discussion.
Should I simply add "r+=lsazman" at the end of the comment line to the patch and upload it,
and just wait after adding "checkin-needed" keyword?
Oh, wait, there does not seem a keyword field in the new bugzilla interface
where I can place "checkin-needed" keyword any longer?
I see "splinter review" link in the attachment section, and there is "check-in" there.
Your guidance is appreciaed.
Flags: needinfo?(lsalzman)
Updated•7 years ago
|
Priority: -- → P3
Whiteboard: [gfx-noted]
Updated•7 years ago
|
Flags: needinfo?(lsalzman)
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9363cd00f8f
Bug Fixes the incorrect use of && in place of &. r=lsalzman
Keywords: checkin-needed
Assignee | ||
Comment 6•7 years ago
|
||
Thank you.
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 7•7 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•