On Mon, Nov 05, 2018 at 11:20:55PM +0200, Kamal Heib wrote:
> On Mon, Nov 05, 2018 at 03:18:02PM -0500, Dennis Dalessandro wrote:
> > On 11/5/2018 6:35 AM, Kamal Heib wrote:
> > > The create_ah verb receive as parameter an udata buffer, this parameter
> > > is missing in rvt_create_ah - fixing that.
> >
> > Please add a bit more to the commit message here. This implies it is fixing
> > a bug but there is no bug per se. rdmavt has no need for udata buffer. If
> > perhaps it is needed because of the on-coming patches, you should probably
> > say this.
> >
> > -Denny
>
> Actually, I think that this is a bug that was found while I was doing the
> changes to support the ib_device_ops in rdmavt. Before my changes the
> initialization of the ib_device verbs in rdmavt was done in the
> check_driver_override() function:
>
> static inline int check_driver_override(struct rvt_dev_info *rdi,
> size_t offset, void *func)
> {
> if (!*(void **)((void *)&rdi->ibdev + offset)) {
> *(void **)((void *)&rdi->ibdev + offset) = func;
> return 0;
> }
>
> return 1;
> }
>
> So, when initialize the create_ah verbs from check_support() using the following
> call, there is no guaranty that create_ah() and rvt_create_ah() have the same
> prototype..., that's why this bug was not found during compilation time.
>
> case CREATE_AH:
> check_driver_override(rdi, offsetof(struct ib_device,
> create_ah),
> rvt_create_ah);
> break;
*BARF* This whole mess must go away as soon as possible.
This patch, with a better commit message, should be stand alone and go
right away. Casting function pointers to different sigantures is a
very bad idea.
Jason
