RFR JDK-8007609

[David Holmes]

On 7/02/2013 9:49 PM, David Holmes wrote:
> On 7/02/2013 8:44 PM, Chris Hegarty wrote:
>> On 02/06/2013 03:02 PM, Alan Bateman wrote:
>>> On 06/02/2013 14:57, John Zavgren wrote:
>>>> Alan: I like your change, but I think the free(result) statement would
>>>> need to be eliminated, otherwise the block a few lines later: /*
>>>> Unable to get final path. */ if(len == 0 && result != NULL) {
>>>> free(result); result = NULL; } would cause result to be freed twice.
>>> Shouldn't do because result = newResult and so will be NULL.
>>>
>>> An alternative would be:
>>>
>>> WCHAR* newResult = (WCHAR*)realloc(...);
>>> if (newResult != NULL) {
>>> result = newResult;
>>> len = (*GetFinalPathNameByHandle_func)( h, result, len, 0);
>>> } else {
>>> len = 0;
>>> }
>>>
>>> which might be clearer.
>>
>> This looks like the best proposed change I've seen on this thread.
>>
>> Also, there may be another potential leak here. I think the 'len == 0 &&
>> ' needs to be removed from the following:
>>
>> 160 /* unable to get final path */
>> 161 if (len == 0 && result != NULL) {
>> 162 free(result);
>> 163 result = NULL;
>> 164 }
>>
>> If the malloc previous to this, "WCHAR *tmp = (WCHAR*)malloc(resultLen *
>> sizeof(WCHAR));", fails it sets len = 0, which will cause result to leak.
>
> AFAICS setting len=0 means len==0 will be true and so we will free(result).

And if len != 0 then we will have already freed result, so avoiding a 
double-free.

David

> David
>
>> -Chris
>>
>>>
>>> -Alan.
>>>