Re: Re: [PATCH v3] RDMA/siw: Fix 64/32bit pointer inconsistency

[Doug Ledford <dledford@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

On Fri, 2019-08-23 at 13:15 -0300, Jason Gunthorpe wrote:
> On Fri, Aug 23, 2019 at 12:13:54PM -0400, Doug Ledford wrote:
> > On Fri, 2019-08-23 at 15:05 +0000, Bernard Metzler wrote:
> > > > > Doug,
> > > > > May I ask you to amend this patch in a way which would
> > > > > just stop this monument of programming stupidity from
> > > > > prolonging into the future, while of course recognizing
> > > > > the impossibility of erasing it from the past?
> > > > > Exchanging the %u with %d would help me regaining
> > > > > some self-confidence ;)
> > > > 
> > > > A
> > > >  q?a:b
> > > > 
> > > > Expression has only a single type. There are some tricky rules
> > > > on
> > > > this, but since gcc does not complain on the %u it means
> > > > 'q?(u32):(int)' is a (u32) and the -1 is implicitly casted.
> > > > 
> > > > The better thing to write would have been U32_MAX instead of -1
> > > > 
> > > 
> > > What I wanted to have though is an easy to spot invalid number
> > > for the QP. This is why I wanted to have it a negative number
> > > on the screen, which is obviously not nicely achievable. So,
> > > yeah, U32_MAX is a better idea. It will not very often be a
> > > valid QP ID...
> > 
> > Given that this patch was still the top of my tree, I fixed this
> > up. 
> > But, I think U32_MAX is wrong.  It should be UINT_MAX (which is what
> > I
> > used).  Otherwise it will give errors on s390 where an int is 31
> > bits
> > (and anywhere else that might have a non-32 bit int).
> 
> qp_id returns u32 and the types of both sides of the : should be
> identical

I disagree.  I would rather the constant in a situation like this be
consistent with the format specifier, which one can see right in the
line of code, than with a type of an element of a struct, which one
would have to go look up.  And the format specifier is %u, unsigned int.

> A non-32 bit int does not exist in Linux, everything would break.

Well, s390 does have a 31 bit limit, but it's just the address space,
not the int size of the registers, so you're right, I was conflating two
different things.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]