Yes, the warning was initially received in the static analyzer SVACE,
the same type as Coverity.
In this case, return value of a function 'get_arm_cp_reginfo' is referenced
at helper.c without checking for ALL, but it is usually checked for this
function (8/9).
________________________________
От: Peter Maydell <peter.mayd...@linaro.org>
Отправлено: 16 октября 2023 г. 19:23
Кому: Миронов Сергей Владимирович
Копия: qemu-devel@nongnu.org; qemu-...@nongnu.org
Тема: Re: [PATCH 1/1] target/arm: Adding a check for the result of calling the
CPU information check function
On Thu, 12 Oct 2023 at 09:57, Sergey Mironov <miro...@fintech.ru> wrote:
>
> 6 out of 7 calls to get_arm_cp_reginfo() are checked
This sounds like it's talking about a Coverity warning, though
it doesn't say so. Is that the motivation here ? If so,
it would be good to say so in the commit message. If not,
the commit message should explain why we're making the change.
That particular Coverity warning is quite prone to false
positives, since it's only a heuristic. Sometimes it's
useful to add an assert(), if it helps both Coverity and
human readers, but not always.
assert()s are also most useful if there's a comment that explains
why we can assume the thing they're assuming, as Alex suggests.
> Signed-off-by: Sergey Mironov <miro...@fintech.ru>
> ---
> target/arm/helper.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 74fbb6e1d7..cffbbaf571 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -198,6 +198,7 @@ static void add_cpreg_to_list(gpointer key, gpointer
> opaque)
> uint32_t regidx = (uintptr_t)key;
> const ARMCPRegInfo *ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
>
> + assert(ri != NULL);
> if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_ALIAS))) {
> cpu->cpreg_indexes[cpu->cpreg_array_len] = cpreg_to_kvm_id(regidx);
> /* The value array need not be initialized at this point */
> --
> 2.31.1
thanks
-- PMM
