Request for review 7198519: Broken build, hotspot-rt win USE_PRECOMPILED_HEADER=0

Coleen Phillimore coleen.phillimore at oracle.com
Thu Oct 4 08:16:28 PDT 2012


This change looks good to me.

On 10/4/2012 9:40 AM, David Holmes wrote:
> On 4/10/2012 10:57 PM, harold seigel wrote:
>> Hi David,
>>
>> Thanks for reviewing my proposed change.
>>
>> 1. I don't know why the other header file #include statements are
>> commented out only for Windows. I suspect that the Windows build worked
>> with these lines commented out. So, the comments were never removed.
>>
>> 2. Even though all platforms appear to need <sys/stat.h>, it is still
>> potentially an operating system dependent file. Hence, its #include
>> statements will still be in operating system dependent files.
>
> FYI <sys/stat.h> is already directly included into system independent 
> files eg share/vm/classfile/classLoader.hpp

Doesn't make it okay...  some of us have worked on crufty old os's that 
don't have sys/stat.h.  :)

Coleen
>
> Cheers,
> David
>
>> Thanks, Harold
>>
>> On 10/3/2012 8:27 PM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> On 4/10/2012 5:43 AM, harold seigel wrote:
>>>> Summary: Broken build, hotspot-rt win USE_PRECOMPILED_HEADER=0
>>>>
>>>> Also deleted include statements that were commented out.
>>>>
>>>>
>>>> open webrev athttp://cr.openjdk.java.net/~coleenp/BUG7198519
>>>>
>>>> bug link athttp://bugs.sun.com/view_bug.do?bug_id=7198519
>>>>
>>>> Tested by successfully building on Windows with USE_PRECOMPILED_HEADER
>>>> turned off. Also, ran tests with build and tested with JPRT.
>>>
>>> This makes jvm_windows.h consistent with the other platforms regarding
>>> <sys/stat.h> and fixes the problem, but it raises a couple of
>>> questions for me:
>>>
>>> 1. Why were the other I/O header file includes commented out only for
>>> windows?
>>> 2. As all platforms require <sys/stat.h> Why do we not simply
>>> uncomment the the include in share/vm/prims/jvm.h
>>>
>>> // HotSpot integration note:
>>> //
>>> // This file and jvm.h used with the JDK are identical,
>>> // except for the three includes removed below
>>>
>>> // #include <sys/stat.h>
>>> // #include "jni.h"
>>> // #include "jvm_md.h"
>>>
>>> and then delete it from the platform-specific headers?
>>>
>>> Cheers,
>>> David


More information about the hotspot-runtime-dev mailing list