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

[Adam Farley8]

Hi All,

Does anyone have an objection to pushing this tiny change in?

It doesn't break anything, it fixes a build break on two supported 
platforms, and it seems
like we never refresh the code from upstream. 

- Adam

> I also advocate the source code fix, as Make isn't meant to use the sort 
of logic required 
> to properly analyse the toolchain version string. 
> 
> e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, 
and Make doesn't
> seem to do substring stuff unless you mess around with shells.
> 
> Plus, as people have said, it's better to solve problem x (or work 
around a specific 
> instance of x) than to ignore the exception, even if the ignoring code 
is toolchain-
> specific.
> 
> - Adam Farley
> 
> > On 2018-03-27 18:44, Phil Race wrote:
> > 
> >> As I said I prefer the make file change, since this is a change to an 
upstream library.
> > 
> > Newtons fourth law: For every reviewer, there's an equal and opposite 
reviewer. :) 
> > 
> > Here I am, advocating a source code fix. As Thomas says, the compiler 
complaint seems reasonable, and disabling it might cause us to miss other 
future errors.
> > 
> > Why can't we push the source code fix, and also submit it upstream?
> > 
> > /Magnus
> > 
> > 
> > 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,
> 
> Returning to this.
> 
> While I understand your reluctance to patch upstream code, let me point 
out that we have not updated libjpeg a single time since the JDK was open 
sourced. We're using 6b from 27-Mar-1998. I had a look at the Wikipedia 
page on libjpeg, and this is the latest "uncontroversial" version of the 
source. Versions 7 and up have proprietary extensions, which in turn has 
resulted in multiple forks of libjpeg. As it stands, it seems unlikely 
that we will ever replace libjpeg 6b with a simple upgrade from upstream.
> 
> I therefore maintain my position that a source code fix would be the 
best way forward here.
> 
> /Magnus
> 
> > 
> > 
> > -phil.
> > 
> > 
> > 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
> > 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> 
> 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU