harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gregory Shimansky <gshiman...@gmail.com>
Subject Re: [drlvm][em64t] drlvm broken on em64t?
Date Tue, 12 Dec 2006 22:55:22 GMT
Geir Magnusson Jr. wrote:
> I found it - comment below :
> 
> Gregory Shimansky wrote:
>>
>> I've created a patch to classlib which fixes the problem with drlvm. 
>> Now if you could test it with IBM VME, I'll commit it:
>>
>> Index: modules/luni/src/main/native/luni/shared/luniglob.c
>> ===================================================================
>> --- modules/luni/src/main/native/luni/shared/luniglob.c (revision 486296)
>> +++ modules/luni/src/main/native/luni/shared/luniglob.c (working copy)
>> @@ -263,24 +263,20 @@
>>
>>      returnCode = properties_load(PORTLIB, propsFile, &props, &number);
>>
>> -    bootstrapClassPath = "";
>> -
>>      if (JNI_OK == returnCode && number != 0)
>>      {
>>          unsigned i = 0;
>>          /* Make a string version of the CP separator */
>>          char cpSeparator[] = {(char)hysysinfo_get_classpathSeparator 
>> (), '\0'};
>>
>>
>> -               /* Read current value of bootclasspath property */
>> +        /* Read current value of bootclasspath property */
>>          rcGetProperty = (*vmInterface)->GetSystemProperty (vmInterface,
>>              BOOTCLASSPATH_PROPERTY,
>>              &bootstrapClassPath);
>>
>> +        // Gregory - no property is found, VM bootclasspath is not 
>> defined
>>          if (VMI_ERROR_NONE != rcGetProperty)
>> -        {
>> -            returnCode = JNI_ERR;
>> -            goto cleanup;
>> -        }
>> +            bootstrapClassPath = NULL;
>>
>>          qsort(props, number, sizeof(key_value_pair), props_compare);
>>
> 
> This puts a bandage on the problem - it seems like we changed the API of 
> GetSystemProperty() because this code clearly expected not to get an 
> error if the property wasn't set.

I've read Oliver's comment to r486100 and it didn't look like this code 
shouldn't expect an error from GetSystemProperty:

======================================
Previously we just replaced whatever was already in the 
org.apache.harmony.boot.class.path property with our own bootclasspath. 
We should not assume that this property is empty before we use it - it 
depends on the VM's bcp initialisation order and how it utilises this 
property internally.
======================================

The "We should not assume" doesn't mean "it should always be this way" 
in my understanding.

> Was this clearly defined behavior?  I'm afraid that we're going to run 
> into this elsewhere...

The bad behavior was in drlvm to crash when property value is not found. 
That was a bug for sure.

-- 
Gregory


Mime
View raw message