Re: [PATCH v1 04/48] x86/insn: Silence -Wshorten-64-to-32 warnings

[Eder Zulian]

Hello, apologies if I'm missing something, but

On Tue, Apr 01, 2025 at 11:23:02AM -0700, Ian Rogers wrote:
> The clang warning -Wshorten-64-to-32 can be useful to catch
> inadvertent truncation. In some instances this truncation can lead to
> changing the sign of a result, for example, truncation to return an
> int to fit a sort routine. Silence the warning by making the implicit
> truncation explicit.
> 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/arch/x86/lib/insn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
> index e91d4c4e1c16..5fa8697498fe 100644
> --- a/tools/arch/x86/lib/insn.c
> +++ b/tools/arch/x86/lib/insn.c
> @@ -92,7 +92,7 @@ static int __insn_get_emulate_prefix(struct insn *insn,
>  			goto err_out;
>  	}
>  
> -	insn->emulate_prefix_size = len;
> +	insn->emulate_prefix_size = (int)len;

wouldn't something like 
+ insn->emulate_prefix_size = !!len;
be preferable in this case?

Apparently the 'insn->emulate_prefix_size' is only used in
'insn_has_emulate_prefix()'. Perhaps both the flag 'emulate_prefix_size'
(currently an 'int') and the 'insn_has_emulate_prefix()' function return
type could be changed to boolean?

>  	insn->next_byte += len;

My knowledge of this part of the code is limited, however IMO it seems
strange to have the 'emulate_prefix_size' flag affected implictly or
explicitly by truncation and the pointer 'insn->next_byte' advanced
regardless.

>  
>  	return 1;
> -- 
> 2.49.0.504.g3bcea36a83-goog
>