[PATCH 1/9] comctl32/button: Use assignment instead of CopyRect

[Gabriel Ivăncescu]

On 12/28/18 21:35, Nikolay Sivov wrote:
> On 12/28/18 3:22 PM, Gabriel Ivăncescu wrote:
>
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>
>> These notes apply to the entire patch series (but each patch is
>> independent).
>>
>> Since patch 156187 was rejected and the reason for lack of including
>> CopyRect
>> in that patch was "it makes no sense. A straight assignment should be
>> used
>> instead", this patch series will do just that, and replace the remaining
>> CopyRect calls with straight assignments (and some NULL checks where
>> needed).
>>
>> To avoid redundant NULL checks, they have been added only to the rects
>> that
>> can actually be NULL.
>
> Even if CopyRect() doesn't necessarily make sense sometimes, it's not
> wrong to use it. I personally don't think such cleanup is useful unless
> you change surrounding code and going for straight assignment without
> null check works better.
>
>>       case TBM_GETTHUMBRECT:
>> -    return CopyRect((LPRECT)lParam, &infoPtr->rcThumb);
>> +        if (!lParam) return FALSE;
>> +        *(RECT*)lParam = infoPtr->rcThumb;
>> +        return TRUE;
> That's an example when it doesn't improve anything.
>
>

Yeah, I understand that, and I agree, but then it should be made inline 
like the others.

I don't get why that patch was rejected in the first place, then. It 
seems totally arbitrary and inconsistent, there's nothing special about 
CopyRect compared to the others, it should also be there.

Hopefully after code freeze and perhaps separated from the PtInRect 
patch, it will be reconsidered... (the inline patch, not this one)