Hi Phil, does that mean I can push the change, or will this happen through jprt?
Best regards, Goetz. > -----Original Message----- > From: Philip Race [mailto:philip.r...@oracle.com] > Sent: Tuesday, December 06, 2016 4:21 AM > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> > Cc: Sergey Bylokhov <sergey.bylok...@oracle.com>; awt- > d...@openjdk.java.net; 2d-dev <2d-dev@openjdk.java.net> > Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> RFR(M): 8170525: Fix minor > issues in awt coding > > I didn't eyeball what you changed but JPRT is now happy. > The build passes on all platforms... > > -phil. > > On 12/5/16, 2:47 PM, Lindenmaier, Goetz wrote: > > Hi Phil, > > > > sorry for that! I fixed it, there is an other occurrence, too. > > I double checked all the casts, there was also a mp_flags<-> mp_sign > wrong in mpi.c > > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/ > > (Also rebased the change) > > > > Best regards, > > Goetz > > > >> -----Original Message----- > >> From: Phil Race [mailto:philip.r...@oracle.com] > >> Sent: Monday, December 05, 2016 7:26 PM > >> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey Bylokhov > >> <sergey.bylok...@oracle.com> > >> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net> > >> Subject: Re: [OpenJDK 2D-Dev]<AWT Dev> RFR(M): 8170525: Fix minor > >> issues in awt coding > >> > >> I tried it .. and just as well I did. It fails in the crypto code on Mac. > >> > >> jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error: > >> expression which evaluates to zero treated as a null pointer constant of > type > >> 'mp_digit *' (aka 'unsigned long *') > >> [-Werror,-Wnon-literal-null-conversion] > >> k.dp = (mp_digit)0; > >> ^~~~~~~~~~~ > >> 1 error generated. > >> > >> -phil. > >> > >> > >> > >> On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote: > >>> Hi Phil, > >>> > >>> that would be great, unfortunately I don't have access to JPRT. > >>> > >>> Thanks, > >>> Goetz. > >>> > >>>> -----Original Message----- > >>>> From: Phil Race [mailto:philip.r...@oracle.com] > >>>> Sent: Friday, December 02, 2016 8:46 PM > >>>> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey > Bylokhov > >>>> <sergey.bylok...@oracle.com> > >>>> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net> > >>>> Subject: Re: [OpenJDK 2D-Dev]<AWT Dev> RFR(M): 8170525: Fix minor > >>>> issues in awt coding > >>>> > >>>> I had no other comments, except that it would be good to be sure > >>>> that this builds on all relevant platforms .. using the 'blessed' > >>>> compilers. > >>>> If you like I can submit a JPRT job for you based on this patch. > >>>> > >>>> -phil. > >>>> > >>>> On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote: > >>>>> Hi Phil, > >>>>> > >>>>> I added the initialization to the other function. > >>>>> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt- > dev/webrev.04/ > >>>>> > >>>>> I missed that because Coverity didn't complain. > >>>>> It's not a perfect tool, but it finds sufficient issues > >>>>> to make it worthwhile. Is the other awt code fine? > >>>>> > >>>>> Best regards, > >>>>> Goetz. > >>>>> > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Phil Race [mailto:philip.r...@oracle.com] > >>>>>> Sent: Donnerstag, 1. Dezember 2016 22:31 > >>>>>> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com>; Sergey > >> Bylokhov > >>>>>> <sergey.bylok...@oracle.com>; Vincent Ryan > >>>> <vincent.x.r...@oracle.com> > >>>>>> Cc: awt-...@openjdk.java.net; 2d-dev<2d-dev@openjdk.java.net>; > >>>> security- > >>>>>> d...@openjdk.java.net > >>>>>> Subject: Re: [OpenJDK 2D-Dev]<AWT Dev> RFR(M): 8170525: Fix > minor > >>>> issues > >>>>>> in awt coding > >>>>>> > >>>>>> Sorry. it is > >>>>>> ops->GetRasInfo(env, ops, lockInfo); > >>>>>> that initialises it .. > >>>>>> > >>>>>> > >>>>>> That is still before the dereference > >>>>>> Anyway, what was the reason for updating one function, but not the > >>>> other. > >>>>>> I don't mind the change, but the inconsistency looks odd. > >>>>>> > >>>>>> -phil. > >>>>>> > >>>>>> On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote: > >>>>>>> Hi Phil, > >>>>>>> > >>>>>>>> Did you actually compile this ? The variable is called "rasBase", not > >>>>>>>> "resBase". > >>>>>>> Yes, I compiled it, and I fixed the error, but that was another repo > >>>>>>> I use for the coverity checks. Somehow it did not find its way into > >>>>>>> the webrev. > >>>>>>> > >>>>>>> I don't see where ops->Lock() initializes this field. > >>>>>>> ops->GetRasInfo(env, ops, lockInfo); does so if it resolves > >>>>>>> to BufImg_GetRasInfo(). I can't look in our code scan results > >>>>>>> because the issue is gone after fixing it, that lists the path > >>>>>>> of execution that leads to the issue. > >>>>>>> If you are sure this is correct I will remove the initialization, > >>>>>>> else I will also put it into the other method. > >>>>>>> > >>>>>>> DBN_GetPixelPointer checks lockInfo->rasBase for NULL and > >>>>>>> does what looks like the error case if it's NULL. Therefore NULL > >>>>>>> seemed a good initialization to me. > >>>>>>> > >>>>>>> Best regards, > >>>>>>> Goetz. > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Phil Race [mailto:philip.r...@oracle.com] > >>>>>>>> Sent: Mittwoch, 30. November 2016 20:59 > >>>>>>>> To: Sergey Bylokhov<sergey.bylok...@oracle.com>; Lindenmaier, > >>>> Goetz > >>>>>>>> <goetz.lindenma...@sap.com>; Vincent Ryan > >>>> <vincent.x.r...@oracle.com> > >>>>>>>> Cc: awt-...@openjdk.java.net; 2d-dev<2d- > d...@openjdk.java.net>; > >>>> security- > >>>>>>>> d...@openjdk.java.net > >>>>>>>> Subject: Re: [OpenJDK 2D-Dev]<AWT Dev> RFR(M): 8170525: Fix > >> minor > >>>>>> issues > >>>>>>>> in awt coding > >>>>>>>> > >>>>>>>> Hi Goetz, > >>>>>>>> > >>>>>>>>> DataBufferNative.c > >>>>>>>>> Using uninitialized value lockInfo.rasBase when calling > >>>>>> DBN_GetPixelPointer. > >>>>>>>> 75 lockInfo.resBase = NULL; > >>>>>>>> > >>>>>>>> Did you actually compile this ? The variable is called "rasBase", not > >>>>>>>> "resBase". > >>>>>>>> > >>>>>>>> And strictly there is no problem since inside DBN_GetPixelPointer > >>>>>>>> the code calls ops->Lock which should initialise this. > >>>>>>>> A "rasBase" of 0 isn't really any better than a random one .. > >>>>>>>> > >>>>>>>> Also I don't see why there's a problem here and not in > >>>>>>>> the function immediately following since it is the exact same case. > >>>>>>>> > >>>>>>>> -phil. > >>>>>>>> > >>>>>>>> On 11/30/2016 10:00 AM, Sergey Bylokhov wrote: > >>>>>>>>> cc 2d-dev. > >>>>>>>>> > >>>>>>>>> On 30.11.16 18:41, Lindenmaier, Goetz wrote: > >>>>>>>>>> Hi Vincent, > >>>>>>>>>> > >>>>>>>>>> thanks for the quit review! > >>>>>>>>>> Good catch that I lost the change to p11_mutex.c ... I had to > >> change > >>>>>>>>>> it and it fell out of my patches. > >>>>>>>>>> I edited the Last Modified Date, and also updated the copyright > >>>>>>>>>> messages. > >>>>>>>>>> New webrev: > >>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/ > >>>>>>>>>> > >>>>>>>>>> Best regards, > >>>>>>>>>> Goetz. > >>>>>>>>>> > >>>>>>>>>> (Am I correct that your openJdk name is Vinnie?) > >>>>>>>>>> > >>>>>>>>>>> -----Original Message----- > >>>>>>>>>>> From: Vincent Ryan [mailto:vincent.x.r...@oracle.com] > >>>>>>>>>>> Sent: Mittwoch, 30. November 2016 14:53 > >>>>>>>>>>> To: Lindenmaier, Goetz<goetz.lindenma...@sap.com> > >>>>>>>>>>> Cc: awt-...@openjdk.java.net; security- > d...@openjdk.java.net > >>>>>>>>>>> Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding > >>>>>>>>>>> > >>>>>>>>>>> Hello Goetz, > >>>>>>>>>>> > >>>>>>>>>>> Please modify the bug summary to reference ECC too. > >>>>>>>>>>> Your ECC changes look fine but the ‘Last Modified Date’ line in > >> the > >>>>>>>>>>> 4 source > >>>>>>>>>>> code headers will need to be updated/added. > >>>>>>>>>>> > >>>>>>>>>>> BTW p11_mutex.c is listed below but appears to be missing > from > >>>> the > >>>>>>>>>>> webrev. > >>>>>>>>>>> > >>>>>>>>>>> Thanks. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On 30 Nov 2016, at 13:12, Lindenmaier, Goetz > >>>>>>>>>>> <goetz.lindenma...@sap.com > >>>> <mailto:goetz.lindenma...@sap.com> > > >>>>>>>> wrote: > >>>>>>>>>>> Hi, > >>>>>>>>>>> > >>>>>>>>>>> I’d like to propose a row of smaller fixes where code is > noted > >>>>>>>>>>> down a > >>>>>>>>>>> bit questionable. > >>>>>>>>>>> SAP’s quality process requires that we fix these in our > internal > >>>>>>>>>>> delivery, > >>>>>>>>>>> and I > >>>>>>>>>>> Would like to share my fixes with openJdk. Some of these > >> fixes > >>>>>>>>>>> are of > >>>>>>>>>>> more > >>>>>>>>>>> theoretical nature as how I understand the code paths > never > >>>>>>>>>>> allow the > >>>>>>>>>>> problematic situation, but fixing it nevertheless assures > that > >>>>>>>>>>> nothing is > >>>>>>>>>>> overseen if the code changes. Most changes are in > >> libawt_xawt, > >>>>>>>>>>> some > >>>>>>>>>>> are in libsunec. > >>>>>>>>>>> > >>>>>>>>>>> I’d appreciate a review: > >>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170525- > >>>> awt/webrev.01/ > >>>>>>>>>>> Changes in detail: > >>>>>>>>>>> > >>>>>>>>>>> awt_InputMethod.c: > >>>>>>>>>>> > >>>>>>>>>>> One might overrun the 100 byte fixed-size string > >>>>>>>>>>> statusWindow->status > >>>>>>>>>>> by copying text->string.multi_byte without checking the length. > >>>>>>>>>>> > >>>>>>>>>>> gtk3_interface.c: > >>>>>>>>>>> > >>>>>>>>>>> This less-than-zero comparison of an unsigned value is > never > >>>> true. > >>>>>>>>>>> Using uninitialized value color. Field color.alpha is > >>>>>>>>>>> uninitialized. > >>>>>>>>>>> E.g. used at gtk3_interface.c:2287. > >>>>>>>>>>> > >>>>>>>>>>> XToolkit.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value ret_timeout. > >>>>>>>>>>> E.g. in XToolkit.c:6809. > >>>>>>>>>>> > >>>>>>>>>>> XWindow.c > >>>>>>>>>>> > >>>>>>>>>>> Argument is incompatible with corresponding format > string > >>>>>>>>>>> conversion. > >>>>>>>>>>> > >>>>>>>>>>> splashscreen_sys.c > >>>>>>>>>>> > >>>>>>>>>>> Overflowed or truncated value (or a value computed from > an > >>>>>>>>>>> overflowed or truncated value) (gdk_scale> 0) ? native_scale * > >>>>>>>>>>> (double)gdk_scale : native_scale used as return value. > >>>>>>>>>>> > >>>>>>>>>>> ec.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value k.dp when calling mp_clear. > >>>>>>>>>>> > >>>>>>>>>>> ecdecode.c > >>>>>>>>>>> > >>>>>>>>>>> You might overrun the 291 byte fixed-size string genenc by > >>>> copying > >>>>>>>>>>> curveParams->geny without checking the length. > >>>>>>>>>>> Added sanity check before doing the string concatenation. > >>>>>>>>>>> > >>>>>>>>>>> ecl_mult.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value kt.flag when calling > >>>>>>>>>>> *group->point_mul. (The > >>>>>>>>>>> function pointer resolves to ec_GF2m_pt_mul_mont.) > >>>>>>>>>>> > >>>>>>>>>>> mpi.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value s. Field s.flag is > >>>>>>>>>>> uninitialized when > >>>>>>>>>>> calling > >>>>>>>>>>> s_mp_exch. > >>>>>>>>>>> Using uninitialized value tmp. Field tmp.flag is > >>>>>>>>>>> uninitialized > >> when > >>>>>>>>>>> calling s_mp_exch > >>>>>>>>>>> Using uninitialized value t.dp when calling mp_clear. > >>>>>>>>>>> > >>>>>>>>>>> p11_mutex.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value *ckpInitArgs. Field ckpInitArgs- > >flags > >> is > >>>>>>>>>>> uninitialized when calling memcpy. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> DataBufferNative.c > >>>>>>>>>>> > >>>>>>>>>>> Using uninitialized value lockInfo.rasBase when calling > >>>>>>>>>>> BN_GetPixelPointer. > >>>>>>>>>>> > >>>>>>>>>>> fontpath.c > >>>>>>>>>>> > >>>>>>>>>>> You might overrun the 512 byte fixed-size string > fontDirPath > >> by > >>>>>>>>>>> copying > >>>>>>>>>>> DirP->name[index] without checking the length. > >>>>>>>>>>>