harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: svn commit: r487015 - /harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp
Date Thu, 14 Dec 2006 09:32:52 GMT
(and the following applies to r487009)

I think you jumped the gun here...

1) This wasn't agreed on dev list.  Clearly there was some consensus, 
but I think that you assumed it to fast... it was only a few hours?  I 
actually am challenging the group to tell me why  (key, NULL) is a bad idea.

2) This requires change in the VM (obviously).  While J9 isn't our VM, 
it's really important to the project to have it working and available 
for us, so this change had to be coordinated - at least discussed - with 
  the community members from IBM.



varlax@apache.org wrote:
> Author: varlax
> Date: Wed Dec 13 22:50:04 2006
> New Revision: 487015
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=487015
> Log:
> Fixed vmi properties handling as agreed on dev list.
> Tested on SUSE9
> 
> Modified:
>     harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp
> 
> Modified: harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp
> URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp?view=diff&rev=487015&r1=487014&r2=487015
> ==============================================================================
> --- harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp (original)
> +++ harmony/enhanced/drlvm/trunk/vm/vmi/src/vmi.cpp Wed Dec 13 22:50:04 2006
> @@ -147,26 +147,17 @@
>  GetSystemProperty(VMInterface *vmi, char *key, char **valuePtr)
>  {
>      char* value = get_property(key, JAVA_PROPERTIES);
> -    if (NULL != value)
> -    {
> -        *valuePtr = strdup(value);
> -        destroy_property_value(value);
> -        return VMI_ERROR_NONE;
> -    }
> -    else
> -    {
> -        *valuePtr = NULL;
> -        return VMI_ERROR_NOT_FOUND;
> -    }
> +    *valuePtr = value ? strdup(value) : NULL;
> +    destroy_property_value(value);
> +    return VMI_ERROR_NONE;
>  }
>  
>  vmiError JNICALL
>  SetSystemProperty(VMInterface *vmi, char *key, char *value)
>  {
> -
> -    /*
> -     * The possible implemenation might be:
> -     */
> +    if (!value || !key) {
> +        return VMI_ERROR_ILLEGAL_ARG;
> +    }
>      set_property(key, value, JAVA_PROPERTIES);
>      return VMI_ERROR_NONE;
>  }
> @@ -193,8 +184,14 @@
>  
>      while(keys[count] != NULL) {
>          char* value = get_property(keys[count], JAVA_PROPERTIES);
> -        iterator((char*)strdup(keys[count]), (char*)strdup(value), userData);
> -        destroy_property_value(value);
> +        /* 
> +         * FIXME: possible inconsistency between iterator and 
> +         * properties count.
> +         */
> +        if (value) {
> +            iterator((char*)strdup(keys[count]), (char*)strdup(value), userData);
> +            destroy_property_value(value);
> +        }
>          count++;
>      }
>      destroy_properties_keys(keys);
> 
> 

Mime
View raw message