On 11/05/2022 01.00, Nikolay Sivov wrote:
On 5/10/22 01:17, Angelo Haller wrote:
+/*** + * DESCRIPTION: + * Sets the state of multiple items for LVS_OWNERDATA listviews. + * Make sure to also disable per item notifications via the notification mask. + * + * PARAMETER(S): + * [I] infoPtr : valid pointer to the listview structure + * [I] nFirst : first item index + * [I] nLast : last item index + * [I] item : item or subitem info + * + * RETURN: + * SUCCESS : TRUE + * FAILURE : FALSE + */
Please remove this header.
I can remove that if so desired. I however do believe a remark to disable per item notifications is important, as this needs to be manually done on all call sites.
+static BOOL LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *infoPtr, INT nFirst, INT nLast, const LVITEMW *item) +{ + NMLVODSTATECHANGE nmlv; + + if (!item) return FALSE; + + ZeroMemory(&nmlv, sizeof(nmlv)); + nmlv.iFrom = nFirst; + nmlv.iTo = nLast; + nmlv.uOldState = 0; + nmlv.uNewState = item->state;
Are state fields always like that? E.g. old is always 0? Can "item" be legitimately NULL?
I did not look into the legitimacy of the existing code of old always being 0. I believe that is the case though. LVN_ODSTATECHANGED seems to be only used for "notify about new selection". Deselection is seemingly always done via "deselect all" (item = -1), as can be seen in the tests I submitted. I have not found any other instances were windows sends LVN_ODSTATECHANGED yet. Regarding "item" being NULL, I do not know if there is a legitimate case for that. I just mimicked the defensive? coding practices I found in LISTVIEW_SetItemState, which does check "item" for NULL.
+ + notify_hdr(infoPtr, LVN_ODSTATECHANGED, (LPNMHDR)&nmlv); + + return TRUE; +} If return value is never going to be used it's better to have it as void.
Again, I am happy to change that. This is, again, just adhering to the existing coding practices found within the file.