On 9/18/18 8:26 PM, Maciej W. Rozycki wrote:
> Hi Fredrik,
>
>> I agree, that is important too. I will post an updated v5 soon. Another
>> alternative change is to define check_insn_opc_user_only as
>>
>> static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
>> {
>> #ifndef CONFIG_USER_ONLY
>> check_insn_opc_removed(ctx, flags);
>> #endif
>> }
>>
>> by referring to check_insn_opc_removed (instead of copying its definition).
>> Would you consider this an improvement for v5 too?
>
> Yes, it does look like an improvement to me, reducing code duplication.
> Thanks for looking into it further.
Yes, also check_insn_opc_removed() is a good place to add
debugging/tracing events.
Fredrik with this change and the swaps, feel free to keep my previous
Reviewed-by tag.
>>> here (and swapping the two former calls ought to be fixed separately; I
>>> haven't checked if there are more cases like this, but if so, then they
>>> would best be amended with a single change).
>>
>> I'll defer other ordering and indentation fixes since I'm not sure whether
>> such changes would be accepted.
>
> Sure, no need for you to rush doing that. In the absence of someone
> willing to do such clean-ups voluntarily I would consider it a
> maintainer's duty really.
>
> Maciej
>
