Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Philip Race
Well I already checked in but I did notice that alignment issue and fixed that before pushing. -phil. On 8/2/16, 3:10 PM, Jim Graham wrote: Also, fix the line of "\" at the right edge of the source and add {} around the body of the if statement... ...jim On 08/02/2016 03:06 PM,

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Jim Graham
I agree as well. Assignments really aren't ever needed inside if statements... ...jim On 08/02/2016 03:06 PM, Vadim Pakhnushev wrote: Or as j = w & 1; if (j != 0) { as in other longer cases. Too much parentheses to my taste. Vadim On 03.08.2016 1:04, Jim Graham

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Vadim Pakhnushev
Or as j = w & 1; if (j != 0) { as in other longer cases. Too much parentheses to my taste. Vadim On 03.08.2016 1:04, Jim Graham wrote: In that case, then I'd write it as "if ((j = (w & 1)) != 0)" to make it clear that the LHS is an assignment. A casual reading of the code might see this as a

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Jim Graham
Also, fix the line of "\" at the right edge of the source and add {} around the body of the if statement... ...jim On 08/02/2016 03:06 PM, Vadim Pakhnushev wrote: Or as j = w & 1; if (j != 0) { as in other longer cases. Too much parentheses to my taste. Vadim On

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Jim Graham
In that case, then I'd write it as "if ((j = (w & 1)) != 0)" to make it clear that the LHS is an assignment. A casual reading of the code might see this as a comparison with an extra set of parentheses until someone counts the equal signs... ...jim On 08/02/2016

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-02 Thread Vadim Pakhnushev
That's what warning said: "place parentheses *around the assignment *to silence this warning" Vadim On 02.08.2016 8:48, Prasanta Sadhukhan wrote: +1. Only one thing, mlib_c_ImageCopy.c#185 do we really need that extra parentheses, ((j = (w & 1))). I guess this should just do if (j = (w &

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-01 Thread Prasanta Sadhukhan
+1. Only one thing, mlib_c_ImageCopy.c#185 do we really need that extra parentheses, ((j = (w & 1))). I guess this should just do if (j = (w & 1)), isn't it? Regards Prasanta On 8/1/2016 10:43 PM, Phil Race wrote: Looking for another "+1" ... -phil. On 07/29/2016 10:13 PM, Vadim Pakhnushev

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-08-01 Thread Phil Race
Looking for another "+1" ... -phil. On 07/29/2016 10:13 PM, Vadim Pakhnushev wrote: Looks good! Vadim On 30.07.2016 6:49, Philip Race wrote: See http://cr.openjdk.java.net/~prr/8074843.1/ Also passes JPRT -phil. On 7/29/16, 7:35 AM, Vadim Pakhnushev wrote: On 29.07.2016 17:30, Philip

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Vadim Pakhnushev
Looks good! Vadim On 30.07.2016 6:49, Philip Race wrote: See http://cr.openjdk.java.net/~prr/8074843.1/ Also passes JPRT -phil. On 7/29/16, 7:35 AM, Vadim Pakhnushev wrote: On 29.07.2016 17:30, Philip Race wrote: On 7/29/16, 7:00 AM, Vadim Pakhnushev wrote: On 29.07.2016 16:28, Philip

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Vadim Pakhnushev
On 29.07.2016 17:30, Philip Race wrote: On 7/29/16, 7:00 AM, Vadim Pakhnushev wrote: On 29.07.2016 16:28, Philip Race wrote: On 7/29/16, 3:23 AM, Vadim Pakhnushev wrote: Phil, I guess you wanted to remove the lines in the Awt2dLibraries.gmk? Ah, yes. Not just disable them Do you

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Philip Race
On 7/29/16, 7:00 AM, Vadim Pakhnushev wrote: On 29.07.2016 16:28, Philip Race wrote: On 7/29/16, 3:23 AM, Vadim Pakhnushev wrote: Phil, I guess you wanted to remove the lines in the Awt2dLibraries.gmk? Ah, yes. Not just disable them Do you think it's worth it to rewrite these

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Vadim Pakhnushev
On 29.07.2016 16:28, Philip Race wrote: On 7/29/16, 3:23 AM, Vadim Pakhnushev wrote: Phil, I guess you wanted to remove the lines in the Awt2dLibraries.gmk? Ah, yes. Not just disable them Do you think it's worth it to rewrite these assignments as separate assignment and a condition?

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Philip Race
On 7/29/16, 3:23 AM, Vadim Pakhnushev wrote: Phil, I guess you wanted to remove the lines in the Awt2dLibraries.gmk? Ah, yes. Not just disable them Do you think it's worth it to rewrite these assignments as separate assignment and a condition? Especially long ones with a lot of

Re: [OpenJDK 2D-Dev] RFR: 8074843: Resolve disabled warnings for libmlib_image and libmlib_image_v

2016-07-29 Thread Vadim Pakhnushev
Phil, I guess you wanted to remove the lines in the Awt2dLibraries.gmk? Do you think it's worth it to rewrite these assignments as separate assignment and a condition? Especially long ones with a lot of parentheses? Like this one, instead of if ((j = ((mlib_s32) ((mlib_addr) psrc_row & 4) >>