Re: [PATCH 7/7] block/ssh.c: Don't double-check that characters are hex digits

[Peter Maydell]

On Wed, 31 Jul 2024 at 16:21, Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 31.07.2024 um 16:36 hat Peter Maydell geschrieben:
> > In compare_fingerprint() we effectively check whether the characters
> > in the fingerprint are valid hex digits twice: first we do so with
> > qemu_isxdigit(), but then the hex2decimal() function also has a code
> > path where it effectively detects an invalid digit and returns -1.
> > This causes Coverity to complain because it thinks that we might use
> > that -1 value in an expression where it would be an integer overflow.
>
> If it's a Coverity issue, I think you want to mention the CID, too.

Yes;

Resolves: Coverity CID 1547813

> > Avoid the double-check of hex digit validity by testing the return
> > values from hex2decimal() rather than doing separate calls to
> > qemu_isxdigit().
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > Could alternatively have put a g_assert_non_reached() in
> > hex2decimal(), but this seemed better to me.
>
> I don't like that hex2decimal() returns -1 when its result is unsigned,
> which is why you had to write the check as > 0xf rather than < 0. So in
> this sense, g_assert_non_reached() would look better to me. But we could
> also just change it to return UINT_MAX instead, which should be the
> same, just written in a less confusing way.

I was not super happy with the -1 either. Happy to change that
to 'return UINT_MAX'.

-- PMM