Closed Bug 1479950 Opened 3 years ago Closed 3 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)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: ishikawa, Assigned: ishikawa)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 1 obsolete file)

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.
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 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-
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)
Attachment #8996591 - Flags: review?(lsalzman) → review+
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)
Priority: -- → P3
Whiteboard: [gfx-noted]
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
Thank you.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.