[1/1] RISC-V: Improve handling of mapping symbols with dot suffix

  From: Tsukasa OI <research_trasio@irq.a4lg.com>

This commit makes minor improvements to mapping symbols (executable)
handling with a dot suffix.

1.  Use size_t instead of int
2.  Allocate minimum size for the architectural string buffer.
3.  memcpy instead of strncpy because we know the exact size to copy.
4.  Minor variable naming changes.

opcodes/ChangeLog:

	* riscv-dis.c (riscv_get_map_state): Minor improvements to
	handling of executable mapping symbols with dot suffix.
---
 opcodes/riscv-dis.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
  

Okay, use size_t looks good.  But I don't suggest changing other
developers' variant naming.  This is usually not a real matter, especially
the naming is already clear enough to understand.

Thanks
Nelson

On Sat, Oct 14, 2023 at 4:37 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:

> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> This commit makes minor improvements to mapping symbols (executable)
> handling with a dot suffix.
>
> 1.  Use size_t instead of int
> 2.  Allocate minimum size for the architectural string buffer.
> 3.  memcpy instead of strncpy because we know the exact size to copy.
> 4.  Minor variable naming changes.
>
> opcodes/ChangeLog:
>
>         * riscv-dis.c (riscv_get_map_state): Minor improvements to
>         handling of executable mapping symbols with dot suffix.
> ---
>  opcodes/riscv-dis.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 216916e9426d..18547d81c20d 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -875,12 +875,12 @@ riscv_get_map_state (int n,
>        char *suffix = strchr (name, '.');
>        if (suffix)
>         {
> -         int suffix_index = (int)(suffix - name);
> -         char *name_substr = xmalloc (suffix_index + 1);
> -         strncpy (name_substr, name, suffix_index);
> -         name_substr[suffix_index] = '\0';
> -         riscv_parse_subset (&riscv_rps_dis, name_substr + 2);
> -         free (name_substr);
> +         size_t arch_len = (size_t) (suffix - name) - 2;
> +         char *arch = xmalloc (arch_len + 1);
> +         memcpy (arch, name + 2, arch_len);
> +         arch[arch_len] = '\0';
> +         riscv_parse_subset (&riscv_rps_dis, arch);
> +         free (arch);
>         }
>        else
>         riscv_parse_subset (&riscv_rps_dis, name + 2);
> --
> 2.42.0
>
>
  

On 2023/10/15 11:38, Nelson Chu wrote:
> Okay, use size_t looks good.  But I don't suggest changing other
> developers' variant naming.  This is usually not a real matter,
> especially the naming is already clear enough to understand.
> 
> Thanks
> Nelson

Well, normally I wouldn't touch that much (if there's only 1. as listed
below) but because I found two possible improvements to the existing
code (2. and 3. as below), I felt that keeping the variable names less
important.

By other changes, "suffix_index" no longer fits the context.
"name_substr" is still valid because this is still a substring of
"name". But since this patch separates *only* the ISA string, just
renaming it "arch" felt clearer.

Thanks,
Tsukasa

> 
> On Sat, Oct 14, 2023 at 4:37 PM Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>> wrote:
> 
>     From: Tsukasa OI <research_trasio@irq.a4lg.com
>     <mailto:research_trasio@irq.a4lg.com>>
> 
>     This commit makes minor improvements to mapping symbols (executable)
>     handling with a dot suffix.
> 
>     1.  Use size_t instead of int
>     2.  Allocate minimum size for the architectural string buffer.
>     3.  memcpy instead of strncpy because we know the exact size to copy.
>     4.  Minor variable naming changes.
> 
>     opcodes/ChangeLog:
> 
>             * riscv-dis.c (riscv_get_map_state): Minor improvements to
>             handling of executable mapping symbols with dot suffix.
>     ---
>      opcodes/riscv-dis.c | 12 ++++++------
>      1 file changed, 6 insertions(+), 6 deletions(-)
> 
>     diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>     index 216916e9426d..18547d81c20d 100644
>     --- a/opcodes/riscv-dis.c
>     +++ b/opcodes/riscv-dis.c
>     @@ -875,12 +875,12 @@ riscv_get_map_state (int n,
>            char *suffix = strchr (name, '.');
>            if (suffix)
>             {
>     -         int suffix_index = (int)(suffix - name);
>     -         char *name_substr = xmalloc (suffix_index + 1);
>     -         strncpy (name_substr, name, suffix_index);
>     -         name_substr[suffix_index] = '\0';
>     -         riscv_parse_subset (&riscv_rps_dis, name_substr + 2);
>     -         free (name_substr);
>     +         size_t arch_len = (size_t) (suffix - name) - 2;
>     +         char *arch = xmalloc (arch_len + 1);
>     +         memcpy (arch, name + 2, arch_len);
>     +         arch[arch_len] = '\0';
>     +         riscv_parse_subset (&riscv_rps_dis, arch);
>     +         free (arch);
>             }
>            else
>             riscv_parse_subset (&riscv_rps_dis, name + 2);
>     -- 
>     2.42.0
>