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

Jason Gunthorpe
2019-08-23T14:42:24Z

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

Jason