Re: [PATCH 2/6] comctl32/listview: Move LVN_ODSTATECHANGED notification to function.

[Nikolay Sivov]

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.
> +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?
> -
> - 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.