Re: [PATCH 1/4] disas: introduce no_raw_bytes

[Helge Deller]

On 3/4/24 20:13, Alex Bennée wrote:
> For plugins we don't expect the raw bytes in the disassembly. We
> already deal with this by hand crafting our capstone call but for
> other diassemblers we need a flag.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/disas/dis-asm.h | 7 +++++++
>   disas/disas.c           | 1 +
>   2 files changed, 8 insertions(+)
>
> diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
> index 2324f6b1a46..5c32e7a310c 100644
> --- a/include/disas/dis-asm.h
> +++ b/include/disas/dis-asm.h
> @@ -396,6 +396,13 @@ typedef struct disassemble_info {
>     /* Command line options specific to the target disassembler.  */
>     char * disassembler_options;
>
> +  /*
> +   * When true instruct the disassembler to not preface opcodes with
> +   * raw bytes. This is mainly for the benefit of the plugin
> +   * interface.
> +   */
> +  bool no_raw_bytes;

Patch in general and idea is OK, but I don't like
the "no_raw_bytes" naming very much.
In patch #2 you use "if (!info->no_raw_bytes) {.."
which is double-negation.

"hide_raw_bytes" is better but still double negation.

Maybe something like "show_opcodes" which defaults to "false"
when used with plugins is better?

Helge

> +
>     /* Field intended to be used by targets in any way they deem suitable.  */
>     void *target_info;
>
> diff --git a/disas/disas.c b/disas/disas.c
> index 0d2d06c2ecc..feb5bc4b665 100644
> --- a/disas/disas.c
> +++ b/disas/disas.c
> @@ -273,6 +273,7 @@ char *plugin_disas(CPUState *cpu, uint64_t addr, size_t 
> size)
>       s.info.buffer_vma = addr;
>       s.info.buffer_length = size;
>       s.info.print_address_func = plugin_print_address;
> +    s.info.no_raw_bytes = true;
>
>       if (s.info.cap_arch >= 0 && cap_disas_plugin(&s.info, addr, size)) {
>           ; /* done */