On 19 mar 2014, at 17:47, Erik Joelsson <erik.joels...@oracle.com> wrote:
> Build part looks good! Thanks. > > Don't forget to push the closed generated configure. I would have forgotten… /Staffan > > /Erik > > On 2014-03-19 15:20, Staffan Larsen wrote: >> Erik, Magnus, >> >> Thanks for the quick looks. Here is an updated version that adds the new >> variable to flags.m4 instead. >> >> http://cr.openjdk.java.net/~sla/8037825/webrev.01/root/ >> http://cr.openjdk.java.net/~sla/8037825/webrev.01/jdk/ >> >> Thanks, >> /Staffan >> >> On 19 mar 2014, at 14:03, Erik Joelsson <erik.joels...@oracle.com> wrote: >> >>> Hello, >>> >>> Nice work! Removing warnings and enforcing them is definitely something we >>> like. >>> >>> For the makefile change, ideally the new variable WARNINGS_ARE_ERRORS >>> should be set in configure. I would suggest flags.m4, in >>> FLAGS_SETUP_COMPILER_FLAGS_MISC. Also, instead of making the conditional on >>> OS, it should be on the new variable TOOLCHAIN_TYPE. Perhaps the variable >>> name should contain the word CFLAGS somewhere to make it more obvious what >>> it applies to. >>> >>> /Erik >>> >>> On 2014-03-19 13:30, Staffan Larsen wrote: >>>> The serviceability libraries (as defined by >>>> jdk/make/lib/ServiceabilityLibraries.gmk) should be compiled with >>>> "warnings as errors". To enable this all current warnings need to be fixed. >>>> >>>> The background for this is that we recently had a bug that could have >>>> easily been avoided if we had paid attention to the warnings. >>>> >>>> webrev: http://cr.openjdk.java.net/~sla/8037825/webrev.00/ >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037825 >>>> >>>> Thanks, >>>> /Staffan >>>> >>>> >>>> Here is a list of warnings that I have fixed: >>>> >>>> linux-i586 >>>> >>>> jdk/src/share/back/eventHandler.c: In function >>>> 'eventHandler_createPermanentInternal': >>>> jdk/src/share/back/eventHandler.c:1685: error: cast from pointer to >>>> integer of different size >>>> jdk/src/share/back/eventHandler.c: In function >>>> 'eventHandler_createInternalThreadOnly': >>>> jdk/src/share/back/eventHandler.c:1694: error: cast from pointer to >>>> integer of different size >>>> >>>> jdk/src/share/back/inStream.c: In function 'inStream_readLong': >>>> jdk/src/share/back/inStream.c:147: error: integer constant is too large >>>> for 'long' type >>>> >>>> >>>> macosx >>>> >>>> jdk/src/share/back/SDE.c:51:1: warning: "true" redefined >>>> jdk/src/share/back/SDE.c:52:1: warning: "false" redefined >>>> >>>> jdk/src/share/back/log_messages.c: In function ‘get_time_stamp’: >>>> jdk/src/share/back/log_messages.c:69: warning: format not a string >>>> literal, argument types not checked >>>> jdk/src/share/back/log_messages.c: In function ‘log_message_end’: >>>> jdk/src/share/back/log_messages.c:174: warning: cast from pointer to >>>> integer of different size >>>> >>>> jdk/src/solaris/native/sun/management/MacosxOperatingSystem.c: In function >>>> 'Java_sun_management_OperatingSystemImpl_getProcessCpuLoad0': >>>> jdk/src/solaris/native/sun/management/MacosxOperatingSystem.c:136: >>>> warning: implicit declaration of function 'JVM_ActiveProcessorCount' >>>> >>>> >>>> windows >>>> >>>> jdk/src/share/back/error_messages.c(328) : warning C4996: '_sleep': This >>>> function or variable has been superceded by newer library or operating >>>> system functionality. Consider using Sleep instead. See online help for >>>> details. >>>> >>>> jdk/src/windows/back/linker_md.c(62) : warning C4013: 'free' undefined; >>>> assuming extern returning int >>>> >>>> jdk/src/share/instrument/PathCharsValidator.c(49) : warning C4267: >>>> 'initializing' : conversion from 'size_t' to 'int', possible loss of data >>>> jdk/src/share/instrument/PathCharsValidator.c(62) : warning C4267: >>>> 'initializing' : conversion from 'size_t' to 'int', possible loss of data >>>> jdk/src/share/instrument/PathCharsValidator.c(179) : warning C4267: '=' : >>>> conversion from 'size_t' to 'int', possible loss of data >>>> >>>> >>>> solaris-x64 >>>> >>>> jdk/src/share/back/inStream.c, line 147: constant promoted to unsigned >>>> long long >
