Re: [Xen-devel] [PATCH v3 01/25] x86emul: make decode_register() return unsigned long *

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.12.17 at 19:32, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/12/17 13:58, Jan Beulich wrote:
>> Quite a few casts can be dropped this way, and type-safeness is being
>> increased by not using void * (same goes for decode_vex_gpr()). Drop
>> casts and no longer needed intermediate variables where possible. Take
>> the opportunity and also switch the last parameter to bool.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> This will need rebasing over 053ae230b1 but that only adjusts the
> parameters type of index so shouldn't cause further problems.

Oh, indeed, I should have checked.

> However, is this wise?  I can certainly see the attraction for not
> needing to casing away from void *, you now give the impression that it
> is safe to deference the returned pointer as an unsigned long, even in
> the cases where it isn't safe.
> 
> At least with returning void*, the required cast highlights that
> something special is going on.

How about this: I drop the last parameter from the function so
that callers outside of the emulator won't be mislead (and we'll
have a new internal function with that parameter kept). Internally
in the emulator we store pointers to long anyway, so the change
here is no net increase of risk getting things wrong. We could
even go as far as keeping the internal function return void *,
but the three places the last argument isn't false/zero the
return value is stored into struct operand's reg field anyway
(which is unsigned long *), so I don't see any value in doing so.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel