On Mon, 2019-08-19 at 13:42 +0530, Selvin Xavier wrote:
> On Fri, Aug 16, 2019 at 5:30 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> > On Fri, Aug 16, 2019 at 10:22:25AM +0530, Selvin Xavier wrote:
> > > On Thu, Aug 15, 2019 at 6:37 PM Jason Gunthorpe <jgg@xxxxxxxx>
> > > wrote:
> > > > On Thu, Aug 15, 2019 at 04:44:37AM -0700, Selvin Xavier wrote:
> > > > > @@ -583,7 +584,7 @@ int bnxt_qplib_create_srq(struct
> > > > > bnxt_qplib_res *res,
> > > > > req.eventq_id = cpu_to_le16(srq->eventq_hw_ring_id);
> > > > >
> > > > > rc = bnxt_qplib_rcfw_send_message(rcfw, (void *)&req,
> > > > > - (void *)&resp, NULL,
> > > > > 0);
> > > > > + (void *)&resp,
> > > > > sizeof(req), NULL, 0);
> > > >
> > > > I really don't like seeing casts to void * in code. Why can't
> > > > you
> > > > properly embed the header in the structs??
> > > Is your objection only in casting to void * or you dont like any
> > > casting here?
> >
> > Explicit cast to void to erase the type is a particularly bad habit
> > that I don't like to see.
> >
> > You'd be better to make the send_message accept void * and the cast
> > inside to the header.
> >
> Ok. Will make this change. But this looks like a for-next item..
> right?
> If so, can you take this patch as is for RC? I will post the change
> mentioned for for-next separately.
This patch is a lot of lines of churn. What creating an array of sizes
such that you could use the cmd value as an index into the array. Then
you'd only need to modify a header file to define the array, and the
send function to lookup the length of the command based upon the command
value itself. Far fewer lines of churn, especially if you are possibly
going to replace this fix in for-next.
--
Doug Ledford <dledford@xxxxxxxxxx>
GPG KeyID: B826A3330E572FDD
Fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Attachment:
signature.asc
Description: This is a digitally signed message part
