On 05/11/13 12:36, Mauro Carvalho Chehab wrote: >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c >>> > > index c85d69da35bd..071596869036 100644 >>> > > --- a/drivers/media/v4l2-core/v4l2-async.c >>> > > +++ b/drivers/media/v4l2-core/v4l2-async.c >>> > > @@ -189,12 +189,14 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) >>> > > struct v4l2_subdev *sd, *tmp; >>> > > unsigned int notif_n_subdev = notifier->num_subdevs; >>> > > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); >>> > > - struct device *dev[n_subdev]; >>> > > + struct device **dev; >>> > > int i = 0; >>> > > >>> > > if (!notifier->v4l2_dev) >>> > > return; >>> > > >>> > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); >>> > > + >> > >> > No check for dev == NULL? > Well, what should be done in this case? > > We could do the changes below: > > void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > { > struct v4l2_subdev *sd, *tmp; > unsigned int notif_n_subdev = notifier->num_subdevs; > unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS); > - struct device *dev[n_subdev]; > + struct device **dev; > int i = 0; > > if (!notifier->v4l2_dev) > return; > > + dev = kmalloc(sizeof(*dev) * n_subdev, GFP_KERNEL); > + if (!dev) { > + WARN_ON(true); > + return; > + } > + > mutex_lock(&list_lock); > > list_del(¬ifier->list); > > list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) { > dev[i] = get_device(sd->dev); > > v4l2_async_cleanup(sd); > > /* If we handled USB devices, we'd have to lock the parent too */ > device_release_driver(dev[i++]); > > if (notifier->unbind) > notifier->unbind(notifier, sd, sd->asd); > } > > mutex_unlock(&list_lock); > > while (i--) { > struct device *d = dev[i]; > > if (d && device_attach(d) < 0) { > const char *name = "(none)"; > int lock = device_trylock(d); > > if (lock && d->driver) > name = d->driver->name; > dev_err(d, "Failed to re-probe to %s\n", name); > if (lock) > device_unlock(d); > } > put_device(d); > } > + kfree(dev); > > notifier->v4l2_dev = NULL; > > /* > * Don't care about the waiting list, it is initialised and populated > * upon notifier registration. > */ > } > EXPORT_SYMBOL(v4l2_async_notifier_unregister); > > But I suspect that this will cause an OOPS anyway, as the device will be > only half-removed. So, it would likely OOPS at device removal or if the > device got probed again, at probing time. > > So, IMHO, we should have at least a WARN_ON() for this case. > > Do you have a better idea? This is how Guennadi's patch looked like when it used dynamic allocation: http://www.spinics.net/lists/linux-sh/msg18194.html -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html