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.