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

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Thu Apr 26 08:39:13 UTC 2018

I don't object, but it's not build code so I don't have the final say.

/Magnus

On 2018-04-25 17:43, Adam Farley8 wrote:

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/ <http://cr.openjdk.java.net/%7Estuefe/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

More information about the build-dev mailing list