A webrev and a code review from the appropriate teams is indeed needed. -kto
On Jan 18, 2012, at 9:56 AM, Phil Race wrote: > Its arguable, whether harmless or not, that each file needs to include it. > Its not unreasonable for an area of the code to have a header file such as > "awt.h" > that is supposed to be the one that's included by the other files in that > area of > the code, and which takes care of common includes. jni_util.h is not > necessarily it. > There isn't a need for every file to include that. > Also many files are 3rd party libs and I don't like editing those as the > changes > really belong upstream. > So a one size fits all approach might be the answer but I'd want to make sure > of that first. > > So I'd like to see the list of files that generate actual warnings as well as > the list > of files that reference malloc but don't include stdlib.h. > > We are well aware that returning int as a default is bad in 64 bit .. I'd > expect > instant death so I'd like to see those actual warnings rather than just the > theoretical ones. > > My grep of a current JDK 8 build log for 64 bit Linux shows the only malloc > warnings > are in hotspot management code. So I am waiting for the proof of the real > problem > > And I can speak for 2d, and if there's 2D files touched I would like to see > any changes > to those files, and the reasoning discussed on 2d-dev .. > > -phil. > > On 1/18/2012 8:26 AM, Kelly O'Hair wrote: >> On Jan 18, 2012, at 12:19 AM, Jonathan Lu wrote: >> >>> Hi core-libs-dev, >>> >>> I found that for some native code of OpenJDK code base, malloc() is used >>> without including header file stdlib.h, such as following files, >>> ./src/solaris/native/sun/java2d/opengl/GLXSurfaceData.c >>> ./src/solaris/native/sun/java2d/x11/XRBackendNative.c >>> .... >>> >>> I assume that there's no hacking tricks involved here, right? because this >>> may cause problem for some C compilers, which assumes 'int' as the default >>> return type of a function if it cannot find the function's declaration >>> during compiling. Under such a condition, actual return result of type >>> 'void*' from malloc() will be converted to 'int', which may result in >>> truncated pointers in 64bit platforms. If the application tries to >>> dereference such a broken pointer, error will occur. >>> >>> Indeed I found some indirect includes of stdlib.h, but there're still some >>> I do not see a stdlib.h get included from any of the direct/indirect >>> included headers. I think in order to fix this problem, two approaches may >>> be considered here, >>> a) add "#include<stdlib.h>" to every missing .c file. >>> b) add "#include<stdlib.h>" to a commonly referenced header file, such as >>> jni_util.h. but it would not be easy to find such a file for all and >>> creating one is the same as approach a). >>> >> I suggest a) It should be harmless and is the right thing to do. >> >> It's been a while since I studied the C standard, but I vaguely recall that >> there was another standard C include file >> that would cause the malloc() prototype declaration to show up. >> Or maybe it wasn't a standard one. In any case, I think your a) approach is >> correct and I don't see much a need >> for detailed discussions on this, as long as it builds correctly with no >> warnings on all platforms. It should be fine and correct. >> That's my 2 cents. >> >> -kto >> >>> But both methods need to change many files, any other ideas about how to >>> fix it more elegantly? >>> >>> Thanks and best regards! >>> - Jonathan >>> >
