6348631 - request for review (updated)

[David Holmes]

Hmmm

coleen phillimore said the following on 12/01/10 04:28:
> This looks good to me.  I have some concerns with the inlined socket 
> functions in:
> 
> http://cr.openjdk.java.net/~ikrylov/6348631.v3/src/os/linux/vm/os_linux.inline.hpp.cdiff.html
> 
> as it causes the .hpp file to import linux system specific hpp files 
> which might cause namespace issues down the line.  I'd rather these 
> sorts of includes be in a .cpp file.  I don't know if they were inlined 
> for performance reasons though, but it seems unlikely to have any benefit.

When I was taught C programming I was taught that each file should be 
self-contained and include all headers that it needed. That way if I 
include a header to get the definitions for the "interface" to some 
library, I don't have to know what all of its dependencies are - I just 
include one header and call the library functions. I don't have to 
discover all its dependencies by trial and error (nor does the provider 
of the library have to document them all).

Arguably for our xxx.inline.hpp files it is a different situation 
because they should only be included by our own xxx.cpp file. But as a 
general rule I would not advocate removing all system includes from all 
hotspot headers.

Cheers,
David

> Everything else looks great - I like how it removes the #include hpi.hpp 
> in files that don't even use hpi.
> 
> Thanks,
> Coleen
> 
> On 11/30/10 06:54, Ivan Krylov wrote:
>>
>> In light of the changes related to the includeDB fix and a few 
>> comments on the previous webrev I am sending a new
>> request for review
>> Webrev: http://cr.openjdk.java.net/~ikrylov/6348631.v3/
>>
>> On 18.11.2010 10:07, Ivan Krylov wrote:
>>> With this fix we are removing the use of the HPI (Host Portable 
>>> interface) library from jvm.
>>>
>>> Webrev: http://cr.openjdk.java.net/~ikrylov/6348631/
>>>
>>>