On Wed, Feb 13, 2019 at 12:45:52PM +0200, Gabriel Ivăncescu wrote:
On 2/13/19 11:50 AM, Huw Davies wrote:
On Tue, Feb 12, 2019 at 03:47:01PM +0200, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
v4: Dumb mistake with get_item_string, sorry for noise.
I've left FindStringPos alone because I'm not 100% sure if a listbox with strings is allowed to have a NULL str (which would give a different code path than currently, if I were to check for NULL from get_item_string instead). LISTBOX_lstrcmpiW ends up using CompareStringW which does check for NULL, so I'm not entirely certain about it.
_InsertString inserts an empty string in this case, but I don't find any tests for this. Could you add one?
There are also many over places (e.g. the 2nd half of _PaintItem) that could use this helper. The goal is to have all reads of ->str go through this helper, even in cases where the code is inside a HAS_STRINGS() block.
Huw.
Ok, I'll see about the test.
Thanks.
I didn't want to mess with PaintItem because it uses the item pointer directly, so presumably it's valid already.
The item variable should start to disappear from most of these functions when they start using the helpers.
Since it's used in so many places, I'll use a local variable to hold it at the beginning (if valid), and get rid of the "item" variable, since I can just check for index instead. Does that sound fine?
Yes, using a local variable to hold the string is absolutely fine.
Huw.