Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties

[Alan Bateman]

Andrew Haley wrote:
> Xueming Shen wrote:
>   
>> Andrew Haley wrote:
>>     
>>>              }
>>>
>>>              if (mapLookup(locale_aliases, temp, &p)) {
>>> -                strcpy(temp, p);
>>> +                temp = realloc(temp, strlen(p)+1);
>>> +                if (temp == NULL) {
>>> +                    JNU_ThrowOutOfMemoryError(env, NULL);
>>> +                    return NULL;
>>> +                }
>>> +        strcpy(temp, p);
>>>   
>>>       
>> The max length string comes back from mapLookup(locale_aliases...) is
>> known (in locale_str.h) , so if we can
>> give temp a minimum size when malloc, for example 64:-) then we might
>> not need to update the code above
>>     
>
> It's fairly clear that this code can be correctly written in a great
> many ways, but among the correct solutions there isn't much reason to
> prefer one over the other.  I will make the change you suggest if it
> is needed to get the patch in.
>
>   
Sherman's suggestion does avoid a realloc when an alias is used but the 
initial sizing would need a comment to help future maintainers. As it 
stands, the current patch should make it obvious to future maintainers 
that the strcpy is safe. Up to you if you want to spend any more time on 
it but either is okay with me.

-Alan.