date:20180531

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

2018-05-31 Thread 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 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

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

2018-05-31 Thread 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 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

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

2018-05-31 Thread 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 
> 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
> > >

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

2018-05-31 Thread Ichiroh Takiguchi


Hello.
8202322 was integrated into jdk-11+15.
I'm using XLC 13.1.3 on AIX 7.1.4.
Build was failed because of "-qvisibility=hidden" on 
make/lib/LibCommon.gmk.

According to "XL C/C++ for AIX 13.1.3" documentation [1],
"-qvisibility=hidden" cannot create shared libraries entry points.
For example, libverify.so was there, but entry points were not resolved 
by "-lverify" option.

I think it should be "-qvisibility=default" (I tried, it worked)
or "-qvisibility=protected" (I had not tried) ?
I'm not familiar with -qvisibility option, but I'd like to find out 
right way.


[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_13.1.3/com.ibm.xlcpp1313.aix.doc/compiler_ref/opt_visibility.html


On 2018-05-16 16:08, Langer, Christoph wrote:

Hi Matthias,

yes, reviewed.

Best regards
Christoph

From: Baesken, Matthias
Sent: Mittwoch, 16. Mai 2018 09:06
To: Langer, Christoph ;
'build-dev@openjdk.java.net' ;
ppc-aix-port-...@openjdk.java.net; core-libs-...@openjdk.java.net
Cc: Lindenmaier, Goetz 
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1


Hi  Christoph can I add you as second reviewer  (other reviewer was
Erik Joelsson) can push the change ?

Best regards, Matthias



From: Langer, Christoph
Sent: Donnerstag, 26. April 2018 16:38
To: Baesken, Matthias
mailto:matthias.baes...@sap.com>>;
'build-dev@openjdk.java.net'
mailto:build-dev@openjdk.java.net>>;
ppc-aix-port-...@openjdk.java.net;
core-libs-...@openjdk.java.net
Cc: Simonis, Volker 
mailto:volker.simo...@sap.com>>
Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on 
xlc 12.1


Hi Matthias,

to me the change in principal looks good.

I'm wondering if it is possible to do a comparison like xlc < 13 (e.g.
extract major number before the first dot, then compare numerically) -
but maybe it is too complicated and the current single version compare
suits our needs ?

Best regards
Christoph

From: Baesken, Matthias
Sent: Donnerstag, 26. April 2018 16:14
To: 'build-dev@openjdk.java.net'
mailto:build-dev@openjdk.java.net>>;
ppc-aix-port-...@openjdk.java.net;
core-libs-...@openjdk.java.net
Cc: Langer, Christoph
mailto:christoph.lan...@sap.com>>; Simonis,
Volker mailto:volker.simo...@sap.com>>
Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 
12.1


Hello  ,  could you please review this small adjustment to  the symbol
visibility compilation settings on AIX ?
Currently  we use  XLC 12.1  to compile  JDK on AIX .

However XLC 12.1   does not support  the "-qvisibility=hidden"
setting currently set on AIX.
It was introduced with  XLC 13.1 . Christoph found some info about it 
here :


https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html

Setting it  only generates  hundreds  of warnings  in the build log ,
warnings look like this :
XlC12.1

bash-4.4$ xlC -qversion
IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
Version: 12.01..0019

bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list
of valid options.

Compare to XLC13.1

bash-3.00$ xlC -qversion
IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07)
Version: 13.01..0008
bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc
bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc


So it is better to avoid  setting these flags when using xlc12.1   .
Please review :

Bug :

https://bugs.openjdk.java.net/browse/JDK-8202322

Change :

http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/


Best regards, Matthias

Re: [8u-dev] RFA for JDK-8079788: Fix broken CL version detection in configure for some Visual Studio configurations

2018-05-31 Thread Seán Coffey


Approved for jdk8u-dev.

regards,
Sean.


On 29/05/2018 15:42, Alexey Ivanov wrote:

I can fix it before pushing.


Regards,
Alexey

On 29/05/2018 15:13, Erik Joelsson wrote:
Looks good (except for my spelling error in the comment "siutations". 
Not sure what the policy is for fixing such in backports.)


/Erik

On 2018-05-29 05:27, Alexey Ivanov wrote:

Hi,

Could you please approve the following backport to 8u-dev?

JBS: https://bugs.openjdk.java.net/browse/JDK-8079788
jdk9 changeset: http://hg.openjdk.java.net/jdk9/dev/rev/d57780478145
Code review: 
http://mail.openjdk.java.net/pipermail/build-dev/2016-August/017566.html 



The patch applies cleanly to jdk8u-dev; generated-configure.sh is 
regenerated.

Webrev: http://cr.openjdk.java.net/~aivanov/8079788/jdk8/webrev.0/

I recently faced this issue, and configure for 8u cannot complete. 
It could be related to backporting of JDK-8042707.


Thank you in advance.

Regards,
Alexey

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

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

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

RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

Re: [8u-dev] RFA for JDK-8079788: Fix broken CL version detection in configure for some Visual Studio configurations

5 matches

Site Navigation

Mail list logo

Footer information