Re: [XEN PATCH 2/4] x86/emulate: move a variable declaration to address MISRA C:2012 Rule 5.3

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.07.2023 17:22, Nicola Vetrini wrote:
> 
> 
> On 27/07/23 17:06, Jan Beulich wrote:
>> On 27.07.2023 12:48, Nicola Vetrini wrote:
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -2024,15 +2024,15 @@ static int cf_check hvmemul_rep_stos(
>>>   
>>>       switch ( p2mt )
>>>       {
>>> -        unsigned long bytes;
>>>           char *buf;
>>>   
>>>       default:
>>>           /* Allocate temporary buffer. */
>>>           for ( ; ; )
>>>           {
>>> -            bytes = *reps * bytes_per_rep;
>>> -            buf = xmalloc_bytes(bytes);
>>> +            unsigned long bytes_tmp;
>>> +            bytes_tmp = *reps * bytes_per_rep;
>>> +            buf = xmalloc_bytes(bytes_tmp);
>>>               if ( buf || *reps <= 1 )
>>>                   break;
>>>               *reps >>= 1;
>>
>> This wants dealing with differently - the outer scope variable is unused
>> (only written to) afaics. Eliminating it will, aiui, address another
>> violation at the same time. And then the same in hvmemul_rep_movs(), just
>> that there the variable itself needs to survive. I guess I'll make a
>> patch ...
> 
> Wouldn't this code at line ~2068 be possibly affected by writing to 
> bytes, if the outer variable is used?

Which outer variable? I'm suggesting to drop that (see the patch that
I've sent already).

Jan

> /* Adjust address for reverse store. */
> if ( df )
>    gpa -= bytes - bytes_per_rep;
> 
> rc = hvm_copy_to_guest_phys(gpa, buf, bytes, curr);
> 
> You're right about the other violation (R2.1)
>