On Mon, Jul 19, 2021 at 06:25:53PM +1000, Alexey Kardashevskiy wrote:
>
>
> 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 <a...@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"?
I don't really mind. I think (-1U) might cause some more Coverity
confusion that ~0U, based on experience with Coverity scans of dtc &
libfdt.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
