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

Thomas Stüfe thomas.stuefe at gmail.com
Thu Apr 26 09:46:40 UTC 2018

Same here. I would like to have this fix in, but do not want to go over Phils head.

I think Phil was the main objector, maybe he could reconsider?`

Thanks, Thomas

On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:

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