On Wed, Jul 14, 2021 at 10:54 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
>
> 在 2021/7/13 下午9:27, Dan Carpenter 写道:
> > On Tue, Jul 13, 2021 at 04:46:55PM +0800, Xie Yongji wrote:
> >> +static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
> >> +{
> >> + struct vduse_vdpa *vdev;
> >> + int ret;
> >> +
> >> + if (dev->vdev)
> >> + return -EEXIST;
> >> +
> >> + vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
> >> + &vduse_vdpa_config_ops, name, true);
> >> + if (!vdev)
> >> + return -ENOMEM;
> > This should be an IS_ERR() check instead of a NULL check.
>
>
> Yes.
>
>
> >
> > The vdpa_alloc_device() macro is doing something very complicated but
> > I'm not sure what. It calls container_of() and that looks buggy until
> > you spot the BUILD_BUG_ON_ZERO() compile time assert which ensures that
> > the container_of() is a no-op.
> >
> > Only one of the callers checks for error pointers correctly so maybe
> > it's too complicated or maybe there should be better documentation.
>
>
> We need better documentation for this macro and fix all the buggy callers.
>
> Yong Ji, want to do that?
>
Sure, I will send the fix soon.
Thanks,
Yongji
