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); - if (strcasecmp(reg1, name) == 0 || - (reg2 && strcasecmp(reg2, name) == 0)) { + g_assert(reg_name[0]); + g_assert(reg_name[1]); + + if (g_ascii_strcasecmp(reg_name[0], name) == 0 || + g_ascii_strcasecmp(reg_name[1], name) == 0) { *val = vals[i]; return true; } @@ -294,12 +295,13 @@ static bool reg_is_u64_fpu(CPURISCVState *env, const char *name, uint64_t *val) } for (int i = 0; i < 32; i++) { - g_autofree char *reg_name = g_strdup(riscv_fpr_regnames[i]); - char *reg1 = strtok(reg_name, "/"); - char *reg2 = strtok(NULL, "/"); + g_autofree char **reg_name = g_strsplit(riscv_fpr_regnames[i], "/", 2); + + g_assert(reg_name[0]); + g_assert(reg_name[1]); - if (strcasecmp(reg1, name) == 0 || - (reg2 && strcasecmp(reg2, name) == 0)) { + if (g_ascii_strcasecmp(reg_name[0], name) == 0 || + g_ascii_strcasecmp(reg_name[1], name) == 0) { *val = env->fpr[i]; return true; } -- 2.51.0
