6348631 - request for review (updated)
David.Holmes at oracle.com
Wed Dec 1 14:15:36 PST 2010
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:
> 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
> Everything else looks great - I like how it removes the #include hpi.hpp
> in files that don't even use hpi.
> 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/
More information about the hotspot-dev