1479950 - 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

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[ISHIKAWA, Chiaki]

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.

Comment 1

[ISHIKAWA, Chiaki]

Attachment: Finxing the incorrect use of "&&".

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.

Comment 2

[Lee Salzman [: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.

Comment 3

[ISHIKAWA, Chiaki]

Attachment: GrColorSpaceXform-int-in-boolean-warning.patch (take 2)

Sorry, I loaded an incorrect patch that was created during source tree shuffle.

This one addresses the correct bug and fixes it.

TIA

Comment 4

[ISHIKAWA, Chiaki]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9363cd00f8f
Bug Fixes the incorrect use of && in place of &. r=lsalzman

Comment 6

[ISHIKAWA, Chiaki]

Thank you.

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a9363cd00f8f