On Wed, 31 Jul 2024 at 16:21, Kevin Wolf <[email protected]> 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 <[email protected]> > > --- > > 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
