RFR: 8074895: os::getenv is inadequate

David Holmes david.holmes at oracle.com
Mon Mar 23 07:45:54 UTC 2015


On 23/03/2015 4:12 PM, David Holmes wrote:
> On 21/03/2015 3:32 AM, Jeremy Manson wrote:
>> Argh.  Yes.  Martin told me not to get involved with Windows, but would
>> I listen?  Of course not...
>>
>> http://cr.openjdk.java.net/~jmanson/8074895/webrev.04/
>
> Looks okay to me - running a test job now.

<sigh> This just isn't meant to be :( It seems that:

GrowableArray<JavaVMOption> options(2, true);

in arguments.cpp is giving the windows compiler some grief:

C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\utilities/growableArray.hpp(171) 
: error C2220: warning treated as error - no 'object' file generated
 
C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\utilities/growableArray.hpp(168) 
: while compiling class template member function 
'GrowableArray<E>::GrowableArray(int,bool,MEMFLAGS)'
         with
         [
             E=JavaVMOption
         ]
 
C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\runtime\arguments.cpp(3516) 
: see reference to class template instantiation 'GrowableArray<E>' being 
compiled
         with
         [
             E=JavaVMOption
         ]
C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\utilities/growableArray.hpp(171) 
: warning C4345: behavior change: an object of POD type constructed with 
an initializer of the form () will be default-initialized
C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\utilities/growableArray.hpp(388) 
: warning C4345: behavior change: an object of POD type constructed with 
an initializer of the form () will be default-initialized
 
C:\jprt\T\P1\071814.daholme\s\hotspot\src\share\vm\utilities/growableArray.hpp(379) 
: while compiling class template member function 'void 
GrowableArray<E>::grow(int)'
         with
         [
             E=JavaVMOption
         ]

I'm guessing it doesn't like the enum as the generic arg, but don't know 
why given that it accepts plain int elsewhere. ???

David
-----


> Coleen: can you re-review the os_windows.cpp changes please.
>
> Thanks,
> David
>
>> On Wed, Mar 18, 2015 at 2:39 PM, David Holmes <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>>     On 19/03/2015 3:04 AM, Jeremy Manson wrote:
>>
>>         Okay.  Here you go:
>>
>>
>> http://cr.openjdk.java.net/~__jmanson/8074895/webrev.03/__index.html
>>
>> <http://cr.openjdk.java.net/~jmanson/8074895/webrev.03/index.html>
>>
>>
>>     !     if (alt_home_dir != NULL)  {
>>     !       strcpy(home_dir, alt_home_dir);
>>     !     } else {
>>
>>     That needs to be strncpy limited by MAX_PATH.
>>
>>     David
>>
>>         Jeremy
>>
>>         On Wed, Mar 18, 2015 at 12:31 AM, David Holmes
>>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>         <mailto:david.holmes at oracle.__com
>>         <mailto:david.holmes at oracle.com>>> wrote:
>>
>>              On 18/03/2015 4:15 PM, Jeremy Manson wrote:
>>
>>                  Oops...  Should I fix it, post a patch, and hope it
>>         compiles?
>>
>>
>>              Please :) I can check the compiles part.
>>
>>              David
>>
>>                  On Tue, Mar 17, 2015 at 9:09 PM, David Holmes
>>                  <david.holmes at oracle.com
>>         <mailto:david.holmes at oracle.com>
>>         <mailto:david.holmes at oracle.__com
>> <mailto:david.holmes at oracle.com>>
>>                  <mailto:david.holmes at oracle.
>>         <mailto:david.holmes at oracle.>____com
>>                  <mailto:david.holmes at oracle.__com
>>         <mailto:david.holmes at oracle.com>>>> wrote:
>>
>>                       Sorry Jeremy, the patch failed on Windows as
>> there is
>>                  another usage
>>                       in os_windows.cpp:
>>
>>                       void os::init_system_properties_______values() {
>>                          // sysclasspath, java_home, dll_dir
>>                          {
>>                            char *home_path;
>>                            char *dll_path;
>>                            char *pslash;
>>                            char *bin = "\\bin";
>>                            char home_dir[MAX_PATH];
>>
>>                            if (!getenv("_ALT_JAVA_HOME_DIR", home_dir,
>>         MAX_PATH)) {
>>
>>                       David
>>
>>
>>                       On 18/03/2015 12:01 PM, David Holmes wrote:
>>
>>                           Submitting via JPRT.
>>
>>                           Thanks,
>>                           David
>>
>>                           On 18/03/2015 4:13 AM, Jeremy Manson wrote:
>>
>>
>>
>>                               On Sun, Mar 15, 2015 at 10:04 PM, David
>> Holmes
>>                               <david.holmes at oracle.com
>>         <mailto:david.holmes at oracle.com>
>>                  <mailto:david.holmes at oracle.__com
>>         <mailto:david.holmes at oracle.com>>
>>                  <mailto:david.holmes at oracle.
>>         <mailto:david.holmes at oracle.>____com
>>         <mailto:david.holmes at oracle.__com
>> <mailto:david.holmes at oracle.com>>>
>>                               <mailto:david.holmes at oracle
>>         <mailto:david.holmes at oracle>.
>>                  <mailto:david.holmes at oracle
>>         <mailto:david.holmes at oracle>.>______com
>>                               <mailto:david.holmes at oracle.
>>         <mailto:david.holmes at oracle.>____com
>>                  <mailto:david.holmes at oracle.__com
>>         <mailto:david.holmes at oracle.com>>>>> wrote:
>>
>>                                    Hi Jeremy,
>>
>>                                    On 14/03/2015 3:00 AM, Jeremy Manson
>>         wrote:
>>
>>                                        Thanks, David!  New rev:
>>
>>         http://cr.openjdk.java.net/~________jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~______jmanson/8074895/webrev.01/>
>>
>>         <http://cr.openjdk.java.net/~______jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/>>
>>
>>
>>         <http://cr.openjdk.java.net/~______jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/>
>>
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.01/>>>
>>
>>
>>
>>         <http://cr.openjdk.java.net/~______jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/>
>>
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.01/>>
>>
>>
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.01/>
>>
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.01/
>>         <http://cr.openjdk.java.net/~jmanson/8074895/webrev.01/>>>>
>>
>>
>>                                    Looks good. Please update copyright
>>         dates in
>>                               memTracker.cpp and
>>                                    vmError.cpp.
>>
>>
>>                               Done.  Thanks for the review, and for
>>         feeding it
>>                  through the
>>                               other
>>                               platforms!
>>
>>         http://cr.openjdk.java.net/~______jmanson/8074895/webrev.02/
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.02/>
>>
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.02/
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.02/>>
>>
>>
>>         <http://cr.openjdk.java.net/~____jmanson/8074895/webrev.02/
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.02/>
>>
>>         <http://cr.openjdk.java.net/~__jmanson/8074895/webrev.02/
>>         <http://cr.openjdk.java.net/~jmanson/8074895/webrev.02/>>>
>>
>>                               Jeremy
>>
>>
>>
>>


More information about the hotspot-runtime-dev mailing list