[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
From: Adam Farley8 <adam.farley () uk ! ibm ! com>
Date: 2018-06-01 8:10:09
Message-ID: OF37428FD7.1B318A84-ON0025829F.002CD2C8-8025829F.002CE101 () notes ! na ! collabserv ! com
[Download RAW message or body]
This is a multipart message in MIME format.
--=_alternative 002CE0858025829F_Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: Quoted-printable
Sounds OK to me. :)
Best Regards
Adam Farley
Philip Race <philip.race@oracle.com> wrote on 01/06/2018 03:04:12:
> From: Philip Race <philip.race@oracle.com>
> To: Adam Farley8 <adam.farley@uk.ibm.com>
> Cc: 2d-dev <2d-dev@openjdk.java.net>, build-dev <build-
> dev@openjdk.java.net>, Andrew Leonard <andrew_m_leonard@uk.ibm.com>,
> "Stuefe, Thomas" <thomas.stuefe@sap.com>
> Date: 01/06/2018 03:04
> Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> compile warning in jchuff.c
>
> > It looks fine to me but I am a bit hazy about who to give
> attribution for the fix ..
>
> I pondered this for a little while and decided it should be
> joint between Adam who identified the issue and championed
> it and Thomas who I think first suggested the code change, modified only
> slightly at Guido's suggestion.
>
> I'll push it tomorrow if every one is OK with that.
>
> -phil.
>
> On 5/31/18, 10:33 AM, Phil Race wrote:
>
> I've grabbed the bug from Thomas and re-opened it
>
> https://bugs.openjdk.java.net/browse/
>
> Your webrev was stripped so I've uploaded here :
>
> http://cr.openjdk.java.net/~prr/8200052/
>
> It looks fine to me but I am a bit hazy about who to give
> attribution for the fix ..
> It is small enough to not require an OCA so there's no issue there
> if we attribute
> it to the IJG guy.
>
> -phil.
> On 05/31/2018 06:31 AM, Adam Farley8 wrote:
> An attachment in the email has been found to contain executable code
> and has been removed.
>
> File removed : webrev.zip, zip,html,js
> Hi Phil,
>
> As requested:
>
>
>
> FYI: I've also contacted Guido, confirmed that the fix worked, and asked
> him to go ahead with merging the fix into his code base.
>
> Best Regards
>
> Adam Farley
>
>
> Phil Race <philip.race@oracle.com> wrote on 30/05/2018 18:06:19:
>
> > From: Phil Race <philip.race@oracle.com>
> > To: Adam Farley8 <adam.farley@uk.ibm.com>
> > Cc: 2d-dev <2d-dev@openjdk.java.net>, Andrew Leonard
> > <andrew_m_leonard@uk.ibm.com>, build-dev <build-
> > dev@openjdk.java.net>, Magnus Ihse Bursie
> > <magnus.ihse.bursie@oracle.com>, "Thomas Stüfe"
<thomas.stuefe@gmail.com>
> > Date: 30/05/2018 18:07
> > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> > compile warning in jchuff.c
> >
> > Thank you for persevering with this. Please submit a webrev with this
> > change .. I think you can leave out the change to jerror.h in the
> jpeg6b case.
> >
> > -phil.
>
> > On 05/30/2018 09:08 AM, Adam Farley8 wrote:
> > Hi Phil,
> >
> > I spoke with the jpegclub rep "Guido", and he was very helpful.
> >
> > He agreed to accept a code change, but recommended an error instead
> > of a while check.
> >
> > ------------------------------ Line 808:
> > < while (bits[j] == 0)
> > < j--;
> > ---
> > > while (bits[j] == 0) {
> > > if (j == 0)
> > > ERREXIT(cinfo, JERR_HUFF_CLEN_OVERFLOW);
> > > j--;
> > > }
> > ------------------------------
> >
> > This makes sense to me, and I verified that it prevents the error.
> >
> > He says:
> > @@@@@@@@@@@@
> > For the release version I would replace the specific
> > JERR_HUFF_CLEN_OVERFLOW by the more general
> > JERR_HUFF_CLEN_OUTOFBOUNDS so that it suits both cases, this
> > requires a change in jerror.h:
> >
> > -JMESSAGE(JERR_HUFF_CLEN_OVERFLOW, "Huffman code size table overflow")
> > +JMESSAGE(JERR_HUFF_CLEN_OUTOFBOUNDS, "Huffman code size table out
> of bounds")
> >
> > The next version (9d) is planned for release in January 2020.
> > A pre-release package will be provided in 2019 on http://
> > jpegclub.org/reference/reference-sources/, I will send you a
notification.
> > @@@@@@@@@@@@
> >
> > While we *could* make the jerror.h change, I suggest we don't for
> > now. If we merge from upstream in January 2020, we'll get that
> > change anyway when the time comes.
> >
> > So what do you say to including the dashed change referenced above
> > to fix our problem now, safe in the knowledge that upstream will be
> > similarly modified (except with the new error type).
> >
> > Best Regards
> >
> > Adam Farley
> >
> > P.S. I'm holding off on giving Guido the green light until after
> > people say if they're happy with the error-enabled version of the fix.
> >
> > Adam Farley8/UK/IBM wrote on 14/05/2018 11:06:28:
> >
> > > From: Adam Farley8/UK/IBM
> > > To: Phil Race <philip.race@oracle.com>
> > > Cc: 2d-dev <2d-dev@openjdk.java.net>, Andrew Leonard
> > > <andrew_m_leonard@uk.ibm.com>, build-dev <build-
> > > dev@openjdk.java.net>, Magnus Ihse Bursie
> > > <magnus.ihse.bursie@oracle.com>, "Thomas Stüfe"
<thomas.stuefe@gmail.com>
> > > Date: 14/05/2018 11:06
> > > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> > > compile warning in jchuff.c
> > >
> > > Hi Phil,
> > >
> > > Would an acceptable compromise be to deliver the source code change
> > > and send the code to the upstream community, allowing them to
include
> > > the fix if/when they are able?
> > >
> > > I believe Magnus was advocating this idea as well. See below.
> > >
> > > Best Regards
> > >
> > > Adam Farley
> > >
> > > > 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@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
itupstream?
> > > > > > >
> > > > > > > /Magnus
> > > > > > >
> > > > > > >
> > > > > > > I've looked at jpeg-9c and it still looks identical to
> whatwe 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
> > appropriatecomment 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
> > seriousobjections
> > > > > > > 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@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
> > > > >
> > > > >
> > >
> > > 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
> 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
--=_alternative 002CE0858025829F_Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset="ISO-8859-1"
<span style=" font-size:10pt;font-family:sans-serif">Sounds OK to me.
> )<br>
<br>
Best Regards<br>
<br>
Adam Farley <br>
</span>
<br><tt><span style=" font-size:10pt">Philip Race <philip.race@oracle.com>
wrote on 01/06/2018 03:04:12:<br>
<br>
> From: Philip Race <philip.race@oracle.com></span></tt>
<br><tt><span style=" font-size:10pt">> To: Adam Farley8 \
<adam.farley@uk.ibm.com></span></tt> <br><tt><span style=" font-size:10pt">> \
Cc: 2d-dev <2d-dev@openjdk.java.net>, build-dev <build-<br>
> dev@openjdk.java.net>, Andrew Leonard \
<andrew_m_leonard@uk.ibm.com>,<br> > "Stuefe, Thomas" \
<thomas.stuefe@sap.com></span></tt> <br><tt><span style=" font-size:10pt">> \
Date: 01/06/2018 03:04</span></tt> <br><tt><span style=" font-size:10pt">> \
Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix <br>
> compile warning in jchuff.c</span></tt>
<br><tt><span style=" font-size:10pt">> <br>
> > It looks fine to me but I am a bit hazy about who to give <br>
> attribution for the fix .. <br>
> <br>
> I pondered this for a little while and decided it should be<br>
> joint between Adam who identified the issue and championed<br>
> it and Thomas who I think first suggested the code change, modified
only<br>
> slightly at Guido's suggestion.<br>
> <br>
> I'll push it tomorrow if every one is OK with that.<br>
> <br>
> -phil.<br>
> <br>
> On 5/31/18, 10:33 AM, Phil Race wrote: </span></tt>
<br><tt><span style=" font-size:10pt">> <br>
> I've grabbed the bug from Thomas and re-opened it<br>
> <br>
> </span></tt><a href=https://bugs.openjdk.java.net/browse/><tt><span style=" \
font-size:10pt">https://bugs.openjdk.java.net/browse/</span></tt></a><tt><span \
style=" font-size:10pt"><br> > <br>
> Your webrev was stripped so I've uploaded here :<br>
> <br>
> </span></tt><a href=http://cr.openjdk.java.net/~prr/8200052/><tt><span style=" \
font-size:10pt">http://cr.openjdk.java.net/~prr/8200052/</span></tt></a><tt><span \
style=" font-size:10pt"><br> > <br>
> It looks fine to me but I am a bit hazy about who to give <br>
> attribution for the fix .. <br>
> It is small enough to not require an OCA so there's no issue there
<br>
> if we attribute<br>
> it to the IJG guy.<br>
> <br>
> -phil.<br>
</span></tt>
<br><tt><span style=" font-size:10pt">> On 05/31/2018 06:31 AM, Adam
Farley8 wrote:</span></tt>
<br><tt><span style=" font-size:10pt">> An attachment in the email has
been found to contain executable code<br>
> and has been removed.<br>
> <br>
> File removed : webrev.zip, zip,html,js<br>
> Hi Phil, <br>
> <br>
> As requested: <br>
> <br>
> <br>
> <br>
> FYI: I've also contacted Guido, confirmed that the fix worked, and
asked <br>
> him to go ahead with merging the fix into his code base.<br>
> <br>
> Best Regards<br>
> <br>
> Adam Farley <br>
> <br>
> <br>
> Phil Race <philip.race@oracle.com> wrote on 30/05/2018 18:06:19:<br>
> <br>
> > From: Phil Race <philip.race@oracle.com> <br>
> > To: Adam Farley8 <adam.farley@uk.ibm.com> <br>
> > Cc: 2d-dev <2d-dev@openjdk.java.net>, Andrew Leonard <br>
> > <andrew_m_leonard@uk.ibm.com>, build-dev <build-<br>
> > dev@openjdk.java.net>, Magnus Ihse Bursie <br>
> > <magnus.ihse.bursie@oracle.com>, "Thomas Stüfe"
<thomas.stuefe@gmail.com> <br>
> > Date: 30/05/2018 18:07 <br>
> > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg:
Fix <br>
> > compile warning in jchuff.c <br>
> > <br>
> > Thank you for persevering with this. Please submit a webrev with
this<br>
> > change .. I think you can leave out the change to jerror.h in
the <br>
> jpeg6b case.<br>
> > <br>
> > -phil.<br>
> <br>
> > On 05/30/2018 09:08 AM, Adam Farley8 wrote: <br>
> > Hi Phil, <br>
> > <br>
> > I spoke with the jpegclub rep "Guido", and he was very
helpful. <br>
> > <br>
> > He agreed to accept a code change, but recommended an error instead
<br>
> > of a while check. <br>
> > <br>
> > ------------------------------ Line 808: <br>
> > < while (bits[j] == 0) <br>
> > < j--; <br>
> > --- <br>
> > > while (bits[j] == 0) { <br>
> > > if (j == 0) <br>
> > > ERREXIT(cinfo, \
JERR_HUFF_CLEN_OVERFLOW); <br>
> > > j--; <br>
> > > } <br>
> > ------------------------------ <br>
> > <br>
> > This makes sense to me, and I verified that it prevents the error.
<br>
> > <br>
> > He says: <br>
> > @@@@@@@@@@@@ <br>
> > For the release version I would replace the specific <br>
> > JERR_HUFF_CLEN_OVERFLOW by the more general <br>
> > JERR_HUFF_CLEN_OUTOFBOUNDS so that it suits both cases, this
<br>
> > requires a change in jerror.h:<br>
> > <br>
> > -JMESSAGE(JERR_HUFF_CLEN_OVERFLOW, "Huffman code size table
overflow")<br>
> > +JMESSAGE(JERR_HUFF_CLEN_OUTOFBOUNDS, "Huffman code size
table out<br>
> of bounds")<br>
> > <br>
> > The next version (9d) is planned for release in January 2020.<br>
> > A pre-release package will be provided in 2019 on http://<br>
> > jpegclub.org/reference/reference-sources/, I will send you a
notification.<br>
> > @@@@@@@@@@@@ <br>
> > <br>
> > While we *could* make the jerror.h change, I suggest we don't
for <br>
> > now. If we merge from upstream in January 2020, we'll get that
<br>
> > change anyway when the time comes. <br>
> > <br>
> > So what do you say to including the dashed change referenced
above <br>
> > to fix our problem now, safe in the knowledge that upstream will
be <br>
> > similarly modified (except with the new error type). <br>
> > <br>
> > Best Regards<br>
> > <br>
> > Adam Farley <br>
> > <br>
> > P.S. I'm holding off on giving Guido the green light until after
<br>
> > people say if they're happy with the error-enabled version of
the fix. <br>
> > <br>
> > Adam Farley8/UK/IBM wrote on 14/05/2018 11:06:28:<br>
> > <br>
> > > From: Adam Farley8/UK/IBM <br>
> > > To: Phil Race <philip.race@oracle.com> <br>
> > > Cc: 2d-dev <2d-dev@openjdk.java.net>, Andrew Leonard
<br>
> > > <andrew_m_leonard@uk.ibm.com>, build-dev <build-<br>
> > > dev@openjdk.java.net>, Magnus Ihse Bursie <br>
> > > <magnus.ihse.bursie@oracle.com>, "Thomas Stüfe"
<thomas.stuefe@gmail.com><br>
> > > Date: 14/05/2018 11:06 <br>
> > > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg:
Fix <br>
> > > compile warning in jchuff.c <br>
> > > <br>
> > > Hi Phil, <br>
> > > <br>
> > > Would an acceptable compromise be to deliver the source
code change <br>
> > > and send the code to the upstream community, allowing them
to include <br>
> > > the fix if/when they are able? <br>
> > > <br>
> > > I believe Magnus was advocating this idea as well. See below.
<br>
> > > <br>
> > > Best Regards <br>
> > > <br>
> > > Adam Farley <br>
> > > <br>
> > > > Same here. I would like to have this fix in, but do
not want to go <br>
> > > > over Phils head. <br>
> > > > <br>
> > > > I think Phil was the main objector, maybe he could
reconsider?` <br>
> > > > <br>
> > > > Thanks, Thomas <br>
> > > > <br>
> > > > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie
<br>
> > > > <magnus.ihse.bursie@oracle.com> wrote: <br>
> > > > > I don't object, but it's not build code so I don't
have the <br>
> final say. <br>
> > > > > <br>
> > > > > /Magnus <br>
> > > > > <br>
> > > > > <br>
> > > > > On 2018-04-25 17:43, Adam Farley8 wrote: <br>
> > > > > <br>
> > > > > Hi All, <br>
> > > > > <br>
> > > > > Does anyone have an objection to pushing this
tiny change in? <br>
> > > > > <br>
> > > > > It doesn't break anything, it fixes a build break
on two supported <br>
> > > > > platforms, and it seems <br>
> > > > > like we never refresh the code from upstream.
<br>
> > > > > <br>
> > > > > - Adam <br>
> > > > > <br>
> > > > >> I also advocate the source code fix, as Make
isn't meant to <br>
> > use the sort <br>
> > > > >> of logic required <br>
> > > > >> to properly analyse the toolchain version
string. <br>
> > > > >> <br>
> > > > >> e.g. An "eq" match on 4.8.5 doesn't
protect the user who is <br>
> > using 4.8.6, <br>
> > > > >> and Make doesn't <br>
> > > > >> seem to do substring stuff unless you mess
around with shells. <br>
> > > > >> <br>
> > > > >> Plus, as people have said, it's better to
solve problem x (or<br>
> > work around <br>
> > > > >> a specific <br>
> > > > >> instance of x) than to ignore the exception,
even if the <br>
> > ignoring code is <br>
> > > > >> toolchain- <br>
> > > > >> specific. <br>
> > > > >> <br>
> > > > >> - Adam Farley <br>
> > > > >> <br>
> > > > >> > On 2018-03-27 18:44, Phil Race wrote:
<br>
> > > > >> > <br>
> > > > >> >> As I said I prefer the make file
change, since this is a <br>
> > change to an <br>
> > > > >> >> upstream library. <br>
> > > > >> > <br>
> > > > >> > Newtons fourth law: For every reviewer,
there's an equal <br>
> and opposite<br>
> > > > >> > reviewer. :) <br>
> > > > >> > <br>
> > > > >> > Here I am, advocating a source code fix.
As Thomas says, <br>
> the compiler<br>
> > > > >> > complaint seems reasonable, and disabling
it might cause us <br>
> > > to miss other <br>
> > > > >> > future errors. <br>
> > > > >> > <br>
> > > > >> > Why can't we push the source code fix,
and also submit itupstream? <br>
> > > > >> > <br>
> > > > >> > /Magnus <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > I've looked at jpeg-9c and it still looks
identical to <br>
> whatwe have in<br>
> > > > >> > 6b, so this code <br>
> > > > >> > seems to have stood the test of time.
I'm also unclear why <br>
> > the compiler <br>
> > > > >> > would <br>
> > > > >> > complain about that and not the one a
few lines later <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > 819 while (bits[i] == 0)
/* find largest <br>
> > > codelength still in <br>
> > > > >> > use */ <br>
> > > > >> > 820 i--; <br>
> > > > >> > <br>
> > > > >> > A push to jchuff.c will get blown away
if/when we upgrade. <br>
> > > > >> > A tool-chain specific fix in the makefile
with an <br>
> > appropriatecomment is <br>
> > > > >> > more targeted. <br>
> > > > >> <br>
> > > > >> Phil, <br>
> > > > >> <br>
> > > > >> Returning to this. <br>
> > > > >> <br>
> > > > >> While I understand your reluctance to patch
upstream code, <br>
> let me point<br>
> > > > >> out that we have not updated libjpeg a single
time since the <br>
> > JDK was open <br>
> > > > >> sourced. We're using 6b from 27-Mar-1998.
I had a look at the <br>
> > > Wikipedia page <br>
> > > > >> on libjpeg, and this is the latest \
"uncontroversial" version of<br>
> > > the source. <br>
> > > > >> Versions 7 and up have proprietary extensions,
which in turn <br>
> > > has resulted in <br>
> > > > >> multiple forks of libjpeg. As it stands, it
seems unlikely that<br>
> > > we will ever <br>
> > > > >> replace libjpeg 6b with a simple upgrade from
upstream. <br>
> > > > >> <br>
> > > > >> I therefore maintain my position that a source
code fix would<br>
> > be the best <br>
> > > > >> way forward here. <br>
> > > > >> <br>
> > > > >> /Magnus <br>
> > > > >> <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > -phil. <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > On 03/27/2018 05:44 AM, Thomas Stüfe
wrote: <br>
> > > > >> > <br>
> > > > >> > Hi all, <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > just a friendly reminder. I would like
to push this tiny <br>
> fix because<br>
> > > > >> > tripping over this on our linux s390x
builds is annoying <br>
> (yes, we can<br>
> > > > >> > maintain patch queues, but this is a
constant error source). <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > I will wait for 24 more hours until a
reaction. If no <br>
> > seriousobjections <br>
> > > > >> > are forcoming, I want to push it (tier1
tests ran thru, and <br>
> > > me and Christoph <br>
> > > > >> > langer are both Reviewers). <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > Thanks! Thomas <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > On Wed, Mar 21, 2018 at 6:20 PM, Thomas
Stüfe <br>
> > <thomas.stuefe@gmail.com> <br>
> > > > >> > wrote: <br>
> > > > >> > <br>
> > > > >> > Hi all, <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > may I please have another review for
this really trivial <br>
> change. It <br>
> > > > >> > fixes a gcc warning on s390 and ppc.
Also, it is probably a <br>
> > > good idea to fix <br>
> > > > >> > this. <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > bug: </span></tt><a \
href="https://bugs.openjdk.java.net/browse/JDK-8200052"><tt><span style=" \
font-size:10pt">https://bugs.openjdk.java.net/browse/JDK-8200052</span></tt></a><tt><span \
style=" font-size:10pt"> <br>
> > > > >> > webrev: <br>
> > > > >> > </span></tt><a \
href="http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-"><tt><span style=" \
font-size:10pt">http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-</span></tt></a><tt><span \
style=" font-size:10pt"><br> > > > warning-in-jchuff.c/webrev.00/webrev/ \
<br> > > > > >> > <br>
> > > > >> > <br>
> > > > >> > This was contributed by Adam Farley at
IBM. <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > I already reviewed this. I also test-built
on zlinux and it works. <br>
> > > > >> > <br>
> > > > >> > <br>
> > > > >> > Thanks, Thomas <br>
> > > > >> > <br>
> > > > >> <br>
> > > > >> Unless stated otherwise above: <br>
> > > > >> IBM United Kingdom Limited - Registered in
England and Wales <br>
> > with number <br>
> > > > >> 741598. <br>
> > > > >> Registered office: PO Box 41, North Harbour,
Portsmouth, <br>
> > > Hampshire PO6 3AU <br>
> > > > >> <br>
> > > > >> <br>
> > > > > <br>
> > > > > Unless stated otherwise above: <br>
> > > > > IBM United Kingdom Limited - Registered in England
and Wales<br>
> with number<br>
> > > > > 741598. <br>
> > > > > Registered office: PO Box 41, North Harbour, Portsmouth,
<br>
> > Hampshire PO6 3AU <br>
> > > > > <br>
> > > > > <br>
> > > <br>
> > > Unless stated otherwise above:<br>
> > > IBM United Kingdom Limited - Registered in England and Wales
with <br>
> > > number 741598. <br>
> > > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6 3AU<br>
> > <br>
> > Unless stated otherwise above:<br>
> > IBM United Kingdom Limited - Registered in England and Wales
with <br>
> > number 741598. <br>
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU<br>
> Unless stated otherwise above:<br>
> IBM United Kingdom Limited - Registered in England and Wales with
<br>
> number 741598. <br>
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
PO6 3AU</span></tt><span style=" font-size:10pt;font-family:sans-serif"><br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU<br>
</span>
--=_alternative 002CE0858025829F_=--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic