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

[Adam Farley8]

Sounds OK to me. :)

Best Regards

Adam Farley 

Philip Race  wrote on 01/06/2018 03:04:12:

> From: Philip Race 
> To: Adam Farley8 
> Cc: 2d-dev <2d-...@openjdk.java.net>, build-dev  d...@openjdk.java.net>, Andrew Leonard ,
> "Stuefe, Thomas" 
> 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  wrote on 30/05/2018 18:06:19:
> 
> > From: Phil Race  
> > To: Adam Farley8  
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
> > , build-dev  > d...@openjdk.java.net>, Magnus Ihse Bursie 
> > , "Thomas Stüfe" 
 
> > 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  
> > > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
> > > , build-dev  > > d...@openjdk.java.net>

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

[Philip Race]

> 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  wrote on 30/05/2018 18:06:19:

> From: Phil Race 
> To: Adam Farley8 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> , build-dev  d...@openjdk.java.net>, Magnus Ihse Bursie
> , "Thomas Stüfe" 


> 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 
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> > , build-dev  > d...@openjdk.java.net>, Magnus Ihse Bursie
> > , "Thomas Stüfe" 


> > 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
> > >  wrote:
> > > > I don't object, but it's not build code so I don't have

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

[Phil Race]

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  wrote on 30/05/2018 18:06:19:

> From: Phil Race 
> To: Adam Farley8 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> , build-dev  d...@openjdk.java.net>, Magnus Ihse Bursie
> , "Thomas Stüfe" 


> 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 
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> > , build-dev  > d...@openjdk.java.net>, Magnus Ihse Bursie
> > , "Thomas Stüfe" 


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

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

[Adam Farley8]

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  wrote on 30/05/2018 18:06:19:

> From: Phil Race 
> To: Adam Farley8 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
> , build-dev  d...@openjdk.java.net>, Magnus Ihse Bursie 
> , "Thomas Stüfe" 

> 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  
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
> > , build-dev  > d...@openjdk.java.net>, Magnus Ihse Bursie 
> > , "Thomas Stüfe" 
 
> > 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 
> > >  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 
> us

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

[Phil Race]

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 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> , build-dev  d...@openjdk.java.net>, Magnus Ihse Bursie
> , "Thomas Stüfe" 


> 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

> >  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.
> > >> >
> > >> &g

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

[Adam Farley8]

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 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
> , build-dev  d...@openjdk.java.net>, Magnus Ihse Bursie 
> , "Thomas Stüfe" 

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

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

[Alex Kashchenko]

Hi,

On 05/21/2018 10:14 AM, Thomas Stüfe wrote:

On Mon, May 21, 2018 at 11:07 AM, John Paul Adrian Glaubitz
 wrote:

On 05/18/2018 07:09 PM, Thomas Stüfe wrote:

The amount of work you have to put into this far outbalances the
amount of work the OpenJDK maintainers would have to spend when (if
ever) they were to merge down upstream libjpeg.


You might have more luck if you replaced libjpeg with libjpeg-turbo
which is used as the default libjpeg implementation in most Linux
distributions these days.



Good idea, but not my call to make


We ship libjpeg-turbo with RH windows builds of OpenJDK. I can say that 
it is not the nicest codebase to build (mostly asm, written for NASM; 
can be built without asm part, but that is not how most people use it). 
While linux distros give it for free, it may be not an optimal variant 
for a portable jpeg impl.


Just my 0.02 .



..Thomas


Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



--
-Alex

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

[Thomas Stüfe]

On Mon, May 21, 2018 at 11:07 AM, John Paul Adrian Glaubitz
 wrote:
> On 05/18/2018 07:09 PM, Thomas Stüfe wrote:
>> The amount of work you have to put into this far outbalances the
>> amount of work the OpenJDK maintainers would have to spend when (if
>> ever) they were to merge down upstream libjpeg.
>
> You might have more luck if you replaced libjpeg with libjpeg-turbo
> which is used as the default libjpeg implementation in most Linux
> distributions these days.
>

Good idea, but not my call to make

..Thomas

> Adrian
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

[Adam Farley8]

Thanks for the compliment, but I plan to leave this open for a little 
while longer.

I have already heard back from the 6x community, and they say they are no 
longer associated with the IJG group (jpegclub.org) that creates 9x.

Unless we see the potential for an upstream merge from the sourceforge 6x 
group, I'll be concentrating exclusively on pushing the change to the IJG 
9x group.

I found some email addresses on the IJG "Site Notice" page, and have 
already emailed the most likely one to try and push the change.

In the meantime, I recommend getting the change into our local copy of the 
code to fix the bug.

Phil, your thoughts?

Best Regards

Adam

> I admire your perseverance :) but I think this is a fools errand.
> 
> The amount of work you have to put into this far outbalances the
> amount of work the OpenJDK maintainers would have to spend when (if
> ever) they were to merge down upstream libjpeg.
> 
> Note that we have a lot of experience with downstream work, since we
> are downstream from OpenJDK - and Oracle JDK before that - since like
> 14 years. Merging upstream with downstream-local patches is part of
> our daily business. That is why I also think that it is quite normal
> to keep local changes downstream and to merge, not just to replace,
> upstream changes. But to do that differently is Oracle's prerogative.
> 
> I for now am resigned to live with local patches (the irony :-) to the
> s390 build. It is annoying but no big deal breaker.
> 
> Thanks & best Regards, Thomas
> 
> 
> On Fri, May 18, 2018 at 5:23 PM, Adam Farley8  
wrote:
> > Hi,
> >
> > I tried to use the IJG's contact page, but no joy. Seems broken; there 
a
> > spinning icon when you hit "send", but nothing happens.
> >
> > 
http://jpegclub.org/reference/contact/
> >
> > So I used a slightly older mailing list on sourceforge. The request to
> > update their code has been sent, and I hope it will appear on the 
mailing
> > list soon.
> >
> > 
https://sourceforge.net/p/libjpeg/mailman/libjpeg-devel-6x/
> >
> > However, that list seems fairly idle too.
> >
> > Does anyone know of another method we can use to communicate this fix
> > upstream?
> >
> > Best Regards
> >
> > Adam
> >
> >> Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile 
warning
> >> in jchuff.cPhilip Race to: Adam Farley8 17/05/2018 03:32
> >> Cc: 2d-dev, Andrew Leonard, build-dev, Magnus Ihse Bursie, "Thomas 
Stüfe"
> >> From: Philip Race 
> >> To: Adam Farley8 
> >> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> >> , build-dev 
,
> >> Magnus Ihse Bursie , "Thomas Stüfe"
> >> 
> >>
> >>
> >> Hi,
> >>
> >> OK .. if you can convince upstream this is worth doing, then we can 
accept
> >> it
> >> as we would not regress when updating. As I noted previously :
> >> 
http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html
> >> this is still an issue in the currently being developed 9c train.
> >>
> >> -phil.
> >>
> >> On 5/14/18, 3:06 AM, Adam Farley8 wrote:
> >> 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
> >> >  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
> >> > 

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

[Thomas Stüfe]

I admire your perseverance :) but I think this is a fools errand.

The amount of work you have to put into this far outbalances the
amount of work the OpenJDK maintainers would have to spend when (if
ever) they were to merge down upstream libjpeg.

Note that we have a lot of experience with downstream work, since we
are downstream from OpenJDK - and Oracle JDK before that - since like
14 years. Merging upstream with downstream-local patches is part of
our daily business. That is why I also think that it is quite normal
to keep local changes downstream and to merge, not just to replace,
upstream changes. But to do that differently is Oracle's prerogative.

I for now am resigned to live with local patches (the irony :-) to the
s390 build. It is annoying but no big deal breaker.

Thanks & best Regards, Thomas


On Fri, May 18, 2018 at 5:23 PM, Adam Farley8  wrote:
> Hi,
>
> I tried to use the IJG's contact page, but no joy. Seems broken; there a
> spinning icon when you hit "send", but nothing happens.
>
> http://jpegclub.org/reference/contact/
>
> So I used a slightly older mailing list on sourceforge. The request to
> update their code has been sent, and I hope it will appear on the mailing
> list soon.
>
> https://sourceforge.net/p/libjpeg/mailman/libjpeg-devel-6x/
>
> However, that list seems fairly idle too.
>
> Does anyone know of another method we can use to communicate this fix
> upstream?
>
> Best Regards
>
> Adam
>
>> Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning
>> in jchuff.cPhilip Race to: Adam Farley8 17/05/2018 03:32
>> Cc: 2d-dev, Andrew Leonard, build-dev, Magnus Ihse Bursie, "Thomas Stüfe"
>> From: Philip Race 
>> To: Adam Farley8 
>> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
>> , build-dev ,
>> Magnus Ihse Bursie , "Thomas Stüfe"
>> 
>>
>>
>> Hi,
>>
>> OK .. if you can convince upstream this is worth doing, then we can accept
>> it
>> as we would not regress when updating. As I noted previously :
>> http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html
>> this is still an issue in the currently being developed 9c train.
>>
>> -phil.
>>
>> On 5/14/18, 3:06 AM, Adam Farley8 wrote:
>> 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
>> >  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 

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

[Adam Farley8]

Hi,

I tried to use the IJG's contact page, but no joy. Seems broken; there a 
spinning icon when you hit "send", but nothing happens.

http://jpegclub.org/reference/contact/

So I used a slightly older mailing list on sourceforge. The request to 
update their code has been sent, and I hope it will appear on the mailing 
list soon.

https://sourceforge.net/p/libjpeg/mailman/libjpeg-devel-6x/

However, that list seems fairly idle too.

Does anyone know of another method we can use to communicate this fix 
upstream?

Best Regards

Adam

> Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile 
warning in jchuff.cPhilip Race to: Adam Farley8 17/05/2018 03:32
> Cc: 2d-dev, Andrew Leonard, build-dev, Magnus Ihse Bursie, "Thomas 
Stüfe"
> From: Philip Race 
> To: Adam Farley8 
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard 
, build-dev , 
Magnus Ihse Bursie , "Thomas Stüfe" 

> 
> 
> Hi,
> 
> OK .. if you can convince upstream this is worth doing, then we can 
accept it
> as we would not regress when updating. As I noted previously :
> http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html
> this is still an issue in the currently being developed 9c train.
> 
> -phil.
> 
> On 5/14/18, 3:06 AM, Adam Farley8 wrote: 
> 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 
> >  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 

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

[Philip Race]

Hi,

OK .. if you can convince upstream this is worth doing, then we can 
accept it

as we would not regress when updating. As I noted previously :
http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html
this is still an issue in the currently being developed 9c train.

-phil.

On 5/14/18, 3:06 AM, Adam Farley8 wrote:

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


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

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

[Adam Farley8]

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

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

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

[Thomas Stüfe]

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

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

[Magnus Ihse Bursie]

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

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

[Adam Farley8]

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

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

[Magnus Ihse Bursie]

On 2018-04-16 12:58, Adam Farley8 wrote:
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  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

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

[Adam Farley8]

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

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

[Adam Farley8]

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

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

[Magnus Ihse Bursie]

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