Hi Christoph, I changed the code in fontpath.c to use snprintf, that seems much better as the concatenated string is a literal anyways.
Also, I fixed the spaces. I'll post a new webrev once I'm through all the reviews. Best regards, Goetz. > -----Original Message----- > From: Langer, Christoph > Sent: Mittwoch, 30. November 2016 20:47 > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> > Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net; 2d- > d...@openjdk.java.net; Vincent Ryan <vincent.x.r...@oracle.com> > Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding > > Hi Goetz, > > I have some small remarks. > > src/java.desktop/unix/native/common/awt/fontpath.c: > > 247 fontDirPath[sizeof(fontDirPath)-1] = '\0'; > -> you should add spaces left and right of '-' > 248 strncat(fontDirPath, "/fonts.dir", sizeof(fontDirPath) - > strlen(fontDirPath)); > -> I think you need to subtract 1 from the 3rd parameter of strncat ('size_t > n') > because man page says: > "If src contains n or more bytes, strncat() writes n+1 bytes to dest (n from > src > plus the terminating null byte). Therefore, the size of dest must be at least > strlen(dest)+n+1." > > src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c: > > add spaces around '-' in the array index > > The rest looks fine to me. > > Best regards > Christoph > > > -----Original Message----- > > From: security-dev [mailto:security-dev-boun...@openjdk.java.net] On > Behalf > > Of Lindenmaier, Goetz > > Sent: Mittwoch, 30. November 2016 16:41 > > To: Vincent Ryan <vincent.x.r...@oracle.com> > > Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net > > Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding > > > > 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-dev@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. > > >