[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:       Phil Race <philip.race () oracle ! com>
Date:       2018-05-31 17:33:46
Message-ID: 64b546b2-c33a-dafd-f284-a2aedac8d730 () oracle ! com
[Download RAW message or body]

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 it 
> upstream?
> > > > >> >
> > > > >> > /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- 
> <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
> > > > >
> > > > >
> > >
> > > 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


[Attachment #3 (text/html)]

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



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic