Request for approval: Backport of 6781583 to hs14/OpenJDK6 (original) (raw)
Kelly O'Hair Kelly.Ohair at Sun.COM
Mon Jun 8 12:49:33 PDT 2009
- Previous message: Request for approval: Backport of 6781583 to hs14/OpenJDK6
- Next message: Request for approval: Backport of 6781583 to hs14/OpenJDK6
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Someone from the hotspot team really needs to review this, but my observations on these changes:
Changing the typedef for jlong seemed really risky to me, but I investigated it more and I'm pretty sure this is the right change, and forces it to match the public definition from jni.h/jni_md.h. So it looks right.
The realpath() changes in src/os/linux/vm/os_linux.cpp seem to change the definition of this method when an error occurs, the control flow has changed, and I'm not exactly sure what the return value is when realpath() fails. I doubt realpath() fails very often here, but won't the buf[] array get some arbitrary directory when realpath() fails? Is that ok? That jvm_path() is a bizarre little method. :^{
Changing the _lineno field in src/share/vm/utilities/vmError.hpp from an int to a size_t might seem right type wise, but sure seems like a waste of space. Does a lineno really need to use up 64bits of space?
The PTR changes look right.
The addition of an unused local variable like in src/share/vm/utilities/ostream.cpp to avoid a warning message just seems wrong to me, and will likely create a new warning message about an unused local variable. Seems like I've had this discussion before. I prefer an explicit (void) cast on a function call whose return value is not needed and I am frustrated with GCC's inability to understand what that (void) cast means. I really like the idea of using -Werror and not allowing warnings, but at times like this I have to wonder if we should just remove the -Werror unless we know that specific version of GCC is < 4.3.
But like I said, a 'real' hotspot developer should probably review this. ;^)
-kto
Andrew John Hughes wrote:
The following webrev:
http://fuseyism.com/6781583/webrev.00/ is backported from OpenJDK7. It allows hs14 to be built using GCC 4.3 and above. Otherwise, the build fails. IcedTea has been applying an equivalent fix as three separately developed patches, two of which have been there pretty much since its inception in the summer of 2007. Ok to commit?
- Previous message: Request for approval: Backport of 6781583 to hs14/OpenJDK6
- Next message: Request for approval: Backport of 6781583 to hs14/OpenJDK6
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]