RFR JDK-8007609

[David Holmes]

On 7/02/2013 10:54 PM, Chris Hegarty wrote:
> On 02/07/2013 11:54 AM, David Holmes wrote:
>> ....
>>> 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.
>
> Here's the code as it stands today.

Yes .... I don't see the problem

>
> 113 result = (WCHAR*)malloc(MAX_PATH * sizeof(WCHAR));
> 114 if (result != NULL) {

we've entered this block so we must free result evetually.

> 115 DWORD len = (*GetFinalPathNameByHandle_func)(h, result, MAX_PATH, 0);
> 116 if (len >= MAX_PATH) {
> 117 /* retry with a buffer of the right size */
> 118 result = (WCHAR*)realloc(result, (len+1) * sizeof(WCHAR));
> 119 if (result != NULL) {
> 120 len = (*GetFinalPathNameByHandle_func)(h, result, len, 0);
> 121 } else {
> 122 len = 0;
> 123 }
> 124 }
> 125 if (len > 0) {

len was good so we've gone this path

> 126 /**
> 127 * Strip prefix (should be \\?\ or \\?\UNC)
> 128 */
> 129 if (result[0] == L'\\' && result[1] == L'\\' &&
> 130 result[2] == L'?' && result[3] == L'\\')
> 131 {
> 132 int isUnc = (result[4] == L'U' &&
> 133 result[5] == L'N' &&
> 134 result[6] == L'C');
> 135 int prefixLen = (isUnc) ? 7 : 4;
> 136 /* actual result length (includes terminator) */
> 137 int resultLen = len - prefixLen + (isUnc ? 1 : 0) + 1;
> 138
> 139 /* copy result without prefix into new buffer */
> 140 WCHAR *tmp = (WCHAR*)malloc(resultLen * sizeof(WCHAR));
> 141 if (tmp == NULL) {
> 142 len = 0; <<<<<<<<<<<<<<<<<<< HERE

malloc failed so we need to bail out. We will now skip to line 161

> 143 } else {
> 144 WCHAR *p = result;
> 145 p += prefixLen;
> 146 if (isUnc) {
> 147 WCHAR *p2 = tmp;
> 148 p2[0] = L'\\';
> 149 p2++;
> 150 wcscpy(p2, p);
> 151 } else {
> 152 wcscpy(tmp, p);
> 153 }
> 154 free(result);
> 155 result = tmp;
> 156 }
> 157 }
> 158 }
> 159
> 160 /* unable to get final path */
> 161 if (len == 0 && result != NULL) {

We reach here because len==0 and result != NULL

> 162 free(result);
> 163 result = NULL;
> 164 }
> 165 }

Looks fine to me.
David

> -Chris.