[OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Thomas Stüfe]

Hi Phil,

On Tue, Mar 27, 2018 at 6:44 PM, Phil Race <philip.race at oracle.com> wrote:

> As I said I prefer the make file change, since this is a change to an
> upstream library.
> I've looked at jpeg-9c and it still looks identical to what we have in 6b,
> so this code
> seems to have stood the test of time. I'm also unclear why the compiler
> would
> complain about that and not the one a few lines later
>
>
>  819   while (bits[i] == 0)          /* find largest codelength still in use */
>  820     i--;
>
> A push to jchuff.c will get blown away if/when we upgrade.
> A tool-chain specific fix in the makefile with an appropriate comment is more targeted.
>
>
> -phil.
>
>
I dislike switching off the compiler warning, since it may be actually
useful. Also I cannot exclude the possibility (I stared at the code some
time already) that the warning is actually meaningful and that we actually
should fix this underflow. Can you?

However, I see your arguments, and since you object to this patch, I revoke
it. Unfortunately I am out of time. I think for the time being we will
continue building with --disable-warnings-as-errors on zlinux.

Thanks, Thomas



>
> On 03/27/2018 05:44 AM, Thomas Stüfe wrote:
>
> Hi all,
>
> just a friendly reminder. I would like to push this tiny fix because
> tripping over this on our linux s390x builds is annoying (yes, we can
> maintain patch queues, but this is a constant error source).
>
> I will wait for 24 more hours until a reaction. If no serious objections
> are forcoming, I want to push it (tier1 tests ran thru, and me and
> Christoph langer are both Reviewers).
>
> Thanks! Thomas
>
> On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> Hi all,
>>
>> may I please have another review for this really trivial change. It fixes
>> a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8200052
>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8200052-
>> fix-warning-in-jchuff.c/webrev.00/webrev/
>>
>> This was contributed by Adam Farley at IBM.
>>
>> I already reviewed this. I also test-built on zlinux and it works.
>>
>> Thanks, Thomas
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180327/4daf0665/attachment-0001.html>