Re: RFR: Even more gcc warnings (8035287)
Thanks for the review Phil! Cheers, Mikael On 2014-02-18 17:37, Phil Race wrote: > Anything else I should do/test? No. Sounds fine. -phil. On 2/18/14 5:30 PM, Mikael Vidstedt wrote: On 2014-02-15 09:31, Phil Race wrote: Looks OK to me although I just realised there's no bug ID here Bug created: https://bugs.openjdk.java.net/browse/JDK-8035287 FWIW I develop on WIndows, Mac & Linux and I've noticed widely divergent things that the compilers on these platforms warn about. Warning free on Linux might not mean warning free on Windows. Also, assuming you develop on Linux might want to check if any of these make the VS compiler less happy about anything. Acknowledged - not all platforms/compilers complain about the same thing(s). I tried my best to manually verify that no new warnings are introduced by building on the usual suspect platforms and grep through the warnings. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. Gosh .. that code must be from 1996 or thereabouts. I hope touching it doesn't mean I own it ;) Anything else I should do/test? Cheers, Mikael -phil. On 2/15/14 8:37 AM, Mikael Vidstedt wrote: Corrected link - this webrev is based on jdk9/client: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/webrev.01/webrev/ Cheers, Mikael On Feb 14, 2014, at 17:54, Mikael Vidstedt wrote: All, A drive-by set of warning fixes: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ Highlights: * src/share/native/com/sun/java/util/jar/pack/bands.cpp Set the size of the array explicitly to increase likelihood of enum and struct array being in sync. Arguably this should be changed to use the (new) [] = instead. Initialize all the fields in the (redundant) terminator struct explicitly. Remove unused macro. * src/share/native/sun/java2d/opengl/OGLContext.c Get the prototype for jio_snprintf from jvm.h to address an implicit declaration. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. * src/solaris/native/sun/awt/awt_LoadLibrary.c The macro is supposed to expand to a void function declaration, but forgets to actually add the "void". Cheers, Mikael
Re: RFR: Even more gcc warnings (8035287)
> Anything else I should do/test? No. Sounds fine. -phil. On 2/18/14 5:30 PM, Mikael Vidstedt wrote: On 2014-02-15 09:31, Phil Race wrote: Looks OK to me although I just realised there's no bug ID here Bug created: https://bugs.openjdk.java.net/browse/JDK-8035287 FWIW I develop on WIndows, Mac & Linux and I've noticed widely divergent things that the compilers on these platforms warn about. Warning free on Linux might not mean warning free on Windows. Also, assuming you develop on Linux might want to check if any of these make the VS compiler less happy about anything. Acknowledged - not all platforms/compilers complain about the same thing(s). I tried my best to manually verify that no new warnings are introduced by building on the usual suspect platforms and grep through the warnings. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. Gosh .. that code must be from 1996 or thereabouts. I hope touching it doesn't mean I own it ;) Anything else I should do/test? Cheers, Mikael -phil. On 2/15/14 8:37 AM, Mikael Vidstedt wrote: Corrected link - this webrev is based on jdk9/client: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/webrev.01/webrev/ Cheers, Mikael On Feb 14, 2014, at 17:54, Mikael Vidstedt wrote: All, A drive-by set of warning fixes: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ Highlights: * src/share/native/com/sun/java/util/jar/pack/bands.cpp Set the size of the array explicitly to increase likelihood of enum and struct array being in sync. Arguably this should be changed to use the (new) [] = instead. Initialize all the fields in the (redundant) terminator struct explicitly. Remove unused macro. * src/share/native/sun/java2d/opengl/OGLContext.c Get the prototype for jio_snprintf from jvm.h to address an implicit declaration. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. * src/solaris/native/sun/awt/awt_LoadLibrary.c The macro is supposed to expand to a void function declaration, but forgets to actually add the "void". Cheers, Mikael
Re: RFR: Even more gcc warnings (8035287)
On 2014-02-15 09:31, Phil Race wrote: Looks OK to me although I just realised there's no bug ID here Bug created: https://bugs.openjdk.java.net/browse/JDK-8035287 FWIW I develop on WIndows, Mac & Linux and I've noticed widely divergent things that the compilers on these platforms warn about. Warning free on Linux might not mean warning free on Windows. Also, assuming you develop on Linux might want to check if any of these make the VS compiler less happy about anything. Acknowledged - not all platforms/compilers complain about the same thing(s). I tried my best to manually verify that no new warnings are introduced by building on the usual suspect platforms and grep through the warnings. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. Gosh .. that code must be from 1996 or thereabouts. I hope touching it doesn't mean I own it ;) Anything else I should do/test? Cheers, Mikael -phil. On 2/15/14 8:37 AM, Mikael Vidstedt wrote: Corrected link - this webrev is based on jdk9/client: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/webrev.01/webrev/ Cheers, Mikael On Feb 14, 2014, at 17:54, Mikael Vidstedt wrote: All, A drive-by set of warning fixes: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ Highlights: * src/share/native/com/sun/java/util/jar/pack/bands.cpp Set the size of the array explicitly to increase likelihood of enum and struct array being in sync. Arguably this should be changed to use the (new) [] = instead. Initialize all the fields in the (redundant) terminator struct explicitly. Remove unused macro. * src/share/native/sun/java2d/opengl/OGLContext.c Get the prototype for jio_snprintf from jvm.h to address an implicit declaration. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. * src/solaris/native/sun/awt/awt_LoadLibrary.c The macro is supposed to expand to a void function declaration, but forgets to actually add the "void". Cheers, Mikael
Re: RFR: Even more gcc warnings
Looks OK to me although I just realised there's no bug ID here FWIW I develop on WIndows, Mac & Linux and I've noticed widely divergent things that the compilers on these platforms warn about. Warning free on Linux might not mean warning free on Windows. Also, assuming you develop on Linux might want to check if any of these make the VS compiler less happy about anything. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. Gosh .. that code must be from 1996 or thereabouts. -phil. On 2/15/14 8:37 AM, Mikael Vidstedt wrote: Corrected link - this webrev is based on jdk9/client: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/webrev.01/webrev/ Cheers, Mikael On Feb 14, 2014, at 17:54, Mikael Vidstedt wrote: All, A drive-by set of warning fixes: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ Highlights: * src/share/native/com/sun/java/util/jar/pack/bands.cpp Set the size of the array explicitly to increase likelihood of enum and struct array being in sync. Arguably this should be changed to use the (new) [] = instead. Initialize all the fields in the (redundant) terminator struct explicitly. Remove unused macro. * src/share/native/sun/java2d/opengl/OGLContext.c Get the prototype for jio_snprintf from jvm.h to address an implicit declaration. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. * src/solaris/native/sun/awt/awt_LoadLibrary.c The macro is supposed to expand to a void function declaration, but forgets to actually add the "void". Cheers, Mikael
Re: RFR: Even more gcc warnings
Corrected link - this webrev is based on jdk9/client: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/webrev.01/webrev/ Cheers, Mikael > On Feb 14, 2014, at 17:54, Mikael Vidstedt wrote: > > > All, > > A drive-by set of warning fixes: > > http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ > > Highlights: > > * src/share/native/com/sun/java/util/jar/pack/bands.cpp > > Set the size of the array explicitly to increase likelihood of enum and > struct array being in sync. Arguably this should be changed to use the (new) > [] = instead. > > Initialize all the fields in the (redundant) terminator struct explicitly. > > Remove unused macro. > > * src/share/native/sun/java2d/opengl/OGLContext.c > > Get the prototype for jio_snprintf from jvm.h to address an implicit > declaration. > > * src/solaris/native/sun/awt/awt_Font.c > > Comparisons with string literals is undefined behavior - keep track of > whether the string should be freed explicitly with a boolean instead. > > * src/solaris/native/sun/awt/awt_LoadLibrary.c > > The macro is supposed to expand to a void function declaration, but forgets > to actually add the "void". > > Cheers, > Mikael >
RFR: Even more gcc warnings
All, A drive-by set of warning fixes: http://cr.openjdk.java.net/~mikael/webrevs/jdk-warnings/jdk-warnings/webrev.00/ Highlights: * src/share/native/com/sun/java/util/jar/pack/bands.cpp Set the size of the array explicitly to increase likelihood of enum and struct array being in sync. Arguably this should be changed to use the (new) [] = instead. Initialize all the fields in the (redundant) terminator struct explicitly. Remove unused macro. * src/share/native/sun/java2d/opengl/OGLContext.c Get the prototype for jio_snprintf from jvm.h to address an implicit declaration. * src/solaris/native/sun/awt/awt_Font.c Comparisons with string literals is undefined behavior - keep track of whether the string should be freed explicitly with a boolean instead. * src/solaris/native/sun/awt/awt_LoadLibrary.c The macro is supposed to expand to a void function declaration, but forgets to actually add the "void". Cheers, Mikael