On 7/26/19 6:36 AM, Richard Henderson wrote:
> On 7/25/19 11:43 PM, tony.ngu...@bt.com wrote:
>> } MemOp;
>>
>> +/* No-op while memory_region_dispatch_[read|write] is converted to MemOp */
>> +#define MEMOP_SIZE(op) (op)/* MemOp to size. */
>> +#define SIZE_MEMOP(ul) (ul)/* Size to MemOp. */
>> +
>
> This doesn't thrill me, because for 9 patches these macros don't do what they
> say on the tin, but I'll accept it.
>
> I would prefer lower-case and that the real implementation in patch 10 be
> inline functions with proper types instead of typeless macros. In particular,
> "unsigned" not "unsigned long" as you imply from "ul" here, since that's what
> was used ...
>
>> MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
>> hwaddr addr,
>> uint64_t *pval,
>> -unsigned size,
>> +MemOp op,
>> MemTxAttrs attrs);
Actually, I thought of something that would make me happier:
Do not make any change to memory_region_dispatch_{read,write} now. Let the
operand continue to be "unsigned size", because it still is, because of the
no-op macros.
Make the change to to the proper type at the same time that you flip the switch
to use the proper conversion function. This will make patch 10 about 5 lines
longer, but we'll have proper typing at all points in between.
r~
>
> ... here.
>
> With the name case change,
> Reviewed-by: Richard Henderson
>
>
> r~
>
