Re: [PATCH qemu] ppc/vof: Fix Coverity issues

[Alexey Kardashevskiy]

On 7/19/21 13:57, David Gibson wrote:
> On Tue, Jul 13, 2021 at 11:46:38PM +1000, Alexey Kardashevskiy wrote:
> > This fixes NEGATIVE_RETURNS, OVERRUN issues reported by the Coverity.
> >
> > This adds a comment about the return parameters number in the VOF hcall.
> > The reason for such counting is to keep the numbers look the same in
> > vof_client_handle() and the Linux (an OF client).
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >
> > Will this make COverity happy? What is the canonical way of fixing these
> > uint32_t vs. int? Thanks,
>
> It might make Coverity happy, but I think it's an ugly approach.
>
> > ---
> >   hw/ppc/vof.c | 12 ++++++++----
> >   1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> > index 81f65962156c..872f671babbe 100644
> > --- a/hw/ppc/vof.c
> > +++ b/hw/ppc/vof.c
> > @@ -517,7 +517,7 @@ static uint32_t vof_instance_to_package(Vof *vof, uint32_t 
> > ihandle)
> >   static uint32_t vof_package_to_path(const void *fdt, uint32_t phandle,
> >                                       uint32_t buf, uint32_t len)
> >   {
> > -    uint32_t ret = -1;
> > +    int ret = -1;
>
> I don't think you want to try to use the same variable for the value
> from phandle_to_path() and the return value from this function -
> they're different types, with different encodings.  The inner value
> should remain int (that's the libfdt convention).
>
> The outer one is explicltly unsigned.  You're not really looking for
> negative error values, but specifically for -1U == ~0U as the single
> error value.  So re-introduce your PROM_ERROR valued, defined as ~0U,
> so that it's clearly unsigned, and use that and unsigned logic for all
> manipulation of the outer value.


Fair enough. One question. Linux defines it as

#define PROM_ERROR (-1u)

Do you still vote for "~0U"?



--
Alexey