Re: [PATCH 1/1] libfdt: incorrect logical constraint in fdt_node_offset_by_phandle()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Mon, Nov 18, 2019 at 04:01:29PM +0100, Heinrich Schuchardt wrote:
> On 11/18/19 12:54 PM, David Gibson wrote:
> > On Mon, Nov 18, 2019 at 08:16:36AM +0100, Heinrich Schuchardt wrote:
> > > A uint32_t value can never be negative. So (phandle == -1) is always false.
> > > 
> > > Add the missing type conversion.
> > > 
> > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
> > 
> > I'd prefer the cast on the constant (or rather, writing it as -1U)
> > instead of on the variable.
> 
> The size of -1U like of any integer is system dependent while uint32_t
> and int32_t are always 32 bit. So using -1U would be incorrect.

Ah, good point.  I'd still prefer (uint32_t)-1 rather than having the
cast on the left.

> 
> Best regards
> 
> Heinrich
> 
> > 
> > > ---
> > > It would be advisable to add -Wextra to the Makefile after correcting
> > > the 50+ -Wsign-compare problems reported.
> > > ---
> > >   libfdt/fdt_ro.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> > > index a5c2797..adab0cb 100644
> > > --- a/libfdt/fdt_ro.c
> > > +++ b/libfdt/fdt_ro.c
> > > @@ -658,7 +658,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle)
> > >   {
> > >   	int offset;
> > > 
> > > -	if ((phandle == 0) || (phandle == -1))
> > > +	if ((phandle == 0) || ((int32_t)phandle == -1))
> > >   		return -FDT_ERR_BADPHANDLE;
> > > 
> > >   	FDT_RO_PROBE(fdt);
> > 
> 

-- 
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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux