RFR JDK-8007609 (original) (raw)

John Zavgren john.zavgren at oracle.com
Fri Feb 8 17:03:14 UTC 2013

Greetings: I posted a new webrev image: http://cr.openjdk.java.net/~jzavgren/8007609/webrev.03/ <http://cr.openjdk.java.net/%7Ejzavgren/8007609/webrev.03/>

The sole "functional" revision is contained in the following small code snippet:

-            /* retry with a buffer of the right size */
-            result = (WCHAR*)realloc(result, (len+1) * sizeof(WCHAR));
-            if (result != NULL) {
-                len = (*GetFinalPathNameByHandle_func)(h, result, len, 0);
-            } else {
+            /* retry the procedure with a buffer of the right size. */
+            WCHAR * newResult  = (WCHAR*)realloc(result, (len+1) * sizeof(WCHAR));
+            if (newResult != NULL) {
+                result = newResult;
+                len = (*GetFinalPathNameByHandle_func)(h, newResult, len, 0);
+            } else

and, the innovation is the use of a local variable to hold the attempted memory reallocation. This makes the code simpler and easier to understand.

I introduced a huge number of additional changes in the file that are my attempt to make the file consistent with our style guidelines.

Changes include: 1.) elimination of tab characters. 2.) spelling, punctuation, and grammar corrections in the comments. 3.) truncation of lines that exceed 80 characters 4.) correction of indentation, line wrapping, etc.

I hope I haven't missed anything. http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html

On 02/08/2013 05:45 AM, Chris Hegarty wrote:

Apologies, you are correct. I'll book an appointment with the optician!

-Chris. On 08/02/2013 00:15, David Holmes wrote: 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(MAXPATH * sizeof(WCHAR)); 114 if (result != NULL) { we've entered this block so we must free result evetually. 115 DWORD len = (*GetFinalPathNameByHandlefunc)(h, result, MAXPATH, 0); 116 if (len >= MAXPATH) { 117 /* retry with a buffer of the right size */ 118 result = (WCHAR*)realloc(result, (len+1) * sizeof(WCHAR)); 119 if (result != NULL) { 120 len = (*GetFinalPathNameByHandlefunc)(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.

-- John Zavgren john.zavgren at oracle.com 603-821-0904 US-Burlington-MA

More information about the core-libs-dev mailing list