On Wed, 22 Oct 2025 at 12:05, Daniel Henrique Barboza
<[email protected]> wrote:
>
> Coverity CID 1641401 reports that, in reg_is_ulong_integer(), we're
> dereferencing a NULL pointer in "reg1" when using it in strcasecmp()
> call. A similar case is reported with CID 1641393.
>
> In theory that will never happen - it's guaranteed that both "reg1" and
> "reg2" is non-NULL because we're retrieving them in compile-time from
> static arrays. Coverity doesn't know that though.
>
> To make Coverity happier and add a bit more clarity in the code,
> g_assert() each token to make it clear that those 2 values aren't
> supposed to be NULL ever. Do that in both reg_is_ulong_integer() and
> reg_is_u64_fpu().
>
> We're also taking the opportunity to implement suggestions made by Peter
> in [1] in both functions:
>
> - use g_strsplit() instead of strtok();
> - use g_ascii_strcasecmp() instead of strcasecmp().
>
> [1] 
> https://lore.kernel.org/qemu-devel/cafeaca_y4bwd9ganbxnpty2mv80vg_jp+a-vkqs5v6f0+bf...@mail.gmail.com/
>
> Coverity: CID 1641393, 1641401
> Fixes: e06d209aa6 ("target/riscv: implement MonitorDef HMP API")
> Suggested-by: Peter Maydell <[email protected]>
> Signed-off-by: Daniel Henrique Barboza <[email protected]>
> ---
>  target/riscv/riscv-qmp-cmds.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> index c499f9b9a7..7bde7090ab 100644
> --- a/target/riscv/riscv-qmp-cmds.c
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -273,12 +273,13 @@ static bool reg_is_ulong_integer(CPURISCVState *env, 
> const char *name,
>      }
>
>      for (int i = 0; i < 32; i++) {
> -        g_autofree char *reg_name = g_strdup(reg_names[i]);
> -        char *reg1 = strtok(reg_name, "/");
> -        char *reg2 = strtok(NULL, "/");
> +        g_autofree char **reg_name = g_strsplit(reg_names[i], "/", 2);

g_autofree frees with g_free(), which isn't the right thing for
freeing an array of pointers. To do autofree here we need

 g_auto(GStrv) reg_name = g_strsplit(...);

(GStrv is the glib type for gchar**; the headers give it an
auto-free mechanism that calls g_strfreev().)

thanks
-- PMM

Reply via email to