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

[Xueming Shen]

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.
>
> Andrew.
>   
It's simply a comment. The first thing popped up when read the code is 
"should we check the length
of p to see if really need to realloc"...then, if we already know the 
maximum length of the string from
aliases lookup why bother to even check the length/realloc each/every 
time. This is kind of nitpicking
comment,  so definitely not going to block your patch.

Sherman