[OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Adam Farley8
adam.farley at uk.ibm.com
Mon Apr 16 10:58:40 UTC 2018
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.
>
>
> 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
More information about the build-dev
mailing list