-----"Jason Gunthorpe" <jgg@xxxxxxxx> wrote: ----- >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx> >From: "Jason Gunthorpe" <jgg@xxxxxxxx> >Date: 08/23/2019 04:42PM >Cc: "Doug Ledford" <dledford@xxxxxxxxxx>, "Leon Romanovsky" ><leon@xxxxxxxxxx>, linux-rdma@xxxxxxxxxxxxxxx, geert@xxxxxxxxxxxxxx >Subject: [EXTERNAL] Re: [PATCH v3] RDMA/siw: Fix 64/32bit pointer >inconsistency > >On Fri, Aug 23, 2019 at 02:33:56PM +0000, Bernard Metzler wrote: >> >> > diff --git a/drivers/infiniband/sw/siw/siw_cm.c >> >> > b/drivers/infiniband/sw/siw/siw_cm.c >> >> > index 9ce8a1b925d2..ae7ea3ad7224 100644 >> >> > +++ b/drivers/infiniband/sw/siw/siw_cm.c >> >> > @@ -355,8 +355,8 @@ static int siw_cm_upcall(struct siw_cep >*cep, >> >> > enum iw_cm_event_type reason, >> >> > getname_local(cep->sock, &event.local_addr); >> >> > getname_peer(cep->sock, &event.remote_addr); >> >> > } >> >> > - siw_dbg_cep(cep, "[QP %u]: id 0x%p, reason=%d, status=%d\n", >> >> > - cep->qp ? qp_id(cep->qp) : -1, id, reason, status); >> >> > + siw_dbg_cep(cep, "[QP %u]: reason=%d, status=%d\n", >> >> > + cep->qp ? qp_id(cep->qp) : -1, reason, status); >> >> ^^^^ >> >> There is a chance that such construction (attempt to print -1 >with >> >%u) >> >> will generate some sort of warning. >> >> >> >> Thanks >> > >> >I didn't see any warnings when I built it. And %u->-1 would be >the >> >same >> >error on 64bit or 32bit, so I think we're safe here. >> > >> >> 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... Thanks Bernard.
