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. I didn't want to mess with PaintItem because it uses the item pointer directly, so presumably it's valid already.
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?