Re: [PATCH 7/7] regedit: Do not get text for items with an index less than one
Hugh McMaster <hugh.mcmaster(a)outlook.com> writes:
@@ -59,7 +59,7 @@ LPWSTR GetItemText(HWND hwndLV, UINT item) { LPWSTR newStr, curStr; unsigned int maxLen = 128; - if (item == 0) return NULL; /* first item is ALWAYS a default */ + if (item < 1) return NULL; /* first item is ALWAYS a default */
How is this different? -- Alexandre Julliard julliard(a)winehq.org
On Wednesday, 31 May 2017 5:23 AM, Alexandre Julliard wrote:
Hugh McMaster writes:
@@ -59,7 +59,7 @@ LPWSTR GetItemText(HWND hwndLV, UINT item) { LPWSTR newStr, curStr; unsigned int maxLen = 128; - if (item == 0) return NULL; /* first item is ALWAYS a default */ + if (item < 1) return NULL; /* first item is ALWAYS a default */
How is this different?
Some of the listview API functions return -1 on failure. If we were to pass in -1 as the item integer to GetItemText(), the function proceeds to allocate memory before attempting to get text for that non-existent item. That memory is not freed in the function and needs to be freed by the caller. We don't currently use the function in this way, but it is quite easy to do so without realizing. So I thought it would be easier to prevent any such use of the function. The commit message probably didn't explain this well at all. I can resend if you feel this is needed, otherwise I'm happy to let it go.
On 31.05.2017 8:49, Hugh McMaster wrote:
On Wednesday, 31 May 2017 5:23 AM, Alexandre Julliard wrote:
Hugh McMaster writes:
@@ -59,7 +59,7 @@ LPWSTR GetItemText(HWND hwndLV, UINT item) { LPWSTR newStr, curStr; unsigned int maxLen = 128; - if (item == 0) return NULL; /* first item is ALWAYS a default */ + if (item < 1) return NULL; /* first item is ALWAYS a default */
How is this different?
Some of the listview API functions return -1 on failure. If we were to pass in -1 as the item integer to GetItemText(), the function proceeds to allocate memory before attempting to get text for that non-existent item.
That memory is not freed in the function and needs to be freed by the caller.
We don't currently use the function in this way, but it is quite easy to do so without realizing. So I thought it would be easier to prevent any such use of the function.
The commit message probably didn't explain this well at all.
I can resend if you feel this is needed, otherwise I'm happy to let it go.
'item' is unsigned, so '< 1' and '== 0' should be equivalent.
On Wednesday, 31 May 2017 3:55 PM, Nikolay Sivov wrote:
'item' is unsigned, so '< 1' and '== 0' should be equivalent.
Yes, they are. But -1 gets wrapped into a positive number, which means the proposed patch doesn't work anyway. We'd have to change the UINT to int, but it's not worth it.
participants (3)
-
Alexandre Julliard -
Hugh McMaster -
Nikolay Sivov