Re: [PATCH] x86/hyperv: Use pointer from memcpy() call for assignment in hv_crash_setup_trampdata()

[Naman Jain <namjain@linux.microsoft.com>]

On 10/31/2025 3:14 PM, Markus Elfring wrote:
> …>> +++ b/arch/x86/hyperv/hv_crash.c
>>> @@ -464,9 +464,7 @@ static int hv_crash_setup_trampdata(u64 trampoline_va)
>>>            return -1;
>>>        }
>>>    -    dest = (void *)trampoline_va;
>>> -    memcpy(dest, &hv_crash_asm32, size);
>>> -
>>> +    dest = memcpy((void *)trampoline_va, &hv_crash_asm32, size);
>>>        dest += size;
>>>        dest = (void *)round_up((ulong)dest, 16);
>>>        tramp = (struct hv_crash_tramp_data *)dest;
>>
>>
>> I tried running spatch Coccinelle checks on this file, but could not get it to flag this improvement.
> 
> The proposed source code transformation is not supported by a coccicheck script so far.
> 
> 
>> Do you mind sharing more details on the issue reproduction please.
> 
> Would you like to take another look at corresponding development discussions?
> 
> Example:
> [RFC] Increasing usage of direct pointer assignments from memcpy() calls with SmPL?
> https://lore.kernel.org/cocci/ddc8654a-9505-451f-87ad-075bfa646381@web.de/
> https://sympa.inria.fr/sympa/arc/cocci/2025-10/msg00049.html
> 
> 
>> I am OK with this change,
> 
> Thanks for a bit of positive feedback.
> 
> 
>> though it may cost code readability a little bit.
> 
> Would you complain about other variable assignments in such a direction?
> 
> Regards,
> Markus

The only thing which concerns readability IMO is that it is based on the 
assumption that the person reading the code is aware of the return value 
of memcpy. Now, it is debatable if that is something which can be 
considered trivial. I don't have a strong opinion there, but would 
prefer it in its current form.

Also, we could have optimized it further by writing it as below, but we 
are not doing that for a reason as we want to keep the code simpler to 
read and understand. The same may apply here as well.

dest = memcpy((void *)trampoline_va, &hv_crash_asm32, size) + size;

Regards,
Naman