On Thu, 5 Nov 2026 15:25:32 GMT, Ioi Lam <[email protected]> wrote: >> We potentially leak a handle in case of failing malloc. >> >> Additionally, GetFinalPathNameByHandleW can fail and return 0. Then we >> should free a newly allocated buffer. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/java.base/windows/native/libjava/canonicalize_md.c line 198: > >> 196: return finalPath; >> 197: } else { // GetFinalPathNameByHandleW failed >> 198: if (newFinalPath != NULL) free(newFinalPath); > > I think it's better to put the error handling immediately after the > `GetFinalPathNameByHandleW` call: > > > DWORD len = GetFinalPathNameByHandleW(h, finalPath, size, 0); > if (len == size) { // failed > CloseHandle(h); > return NULL; > }
I think `len >= size` is a failure, not only len == size. See the documentation of GetFinalPathNameByHandleW https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfinalpathnamebyhandlew 'If the function fails because lpszFilePath is too small to hold the string plus the terminating null character, the return value is the required buffer size, in TCHARs. This value includes the size of the terminating null character.' And in case of failure we try again with a larger malloced buffer. Why should we immediately return? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33752#discussion_r4482560493
