Hi Hans, Thanks for the comments! Hans Verkuil wrote:
On 10/02/13 15:45, Sakari Ailus wrote:Dequeueing events was is entirely possible even if none are subscribed, leading to sleeping indefinitely. Fix this by returning -ENOENT when no events are subscribed. Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> --- drivers/media/v4l2-core/v4l2-event.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-event.c b/drivers/media/v4l2-core/v4l2-event.c index b53897e..553a800 100644 --- a/drivers/media/v4l2-core/v4l2-event.c +++ b/drivers/media/v4l2-core/v4l2-event.c @@ -77,10 +77,17 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event, mutex_unlock(fh->vdev->lock); do { - ret = wait_event_interruptible(fh->wait, - fh->navailable != 0); + bool subscribed;Can you add an empty line here?
Sure.
+ ret = wait_event_interruptible( + fh->wait, + fh->navailable != 0 || + !(subscribed = v4l2_event_has_subscribed(fh))); if (ret < 0) break; + if (!subscribed) { + ret = -EIO;Shouldn't this be -ENOENT?
If I use -ENOENT, having no events subscribed is indistinguishable form no events pending condition. Combine that with using select(2), and you can no longer distinguish having no events subscribed from the case where you got an event but someone else (another thread or process) dequeued it.
-EIO makes that explicit --- this also mirrors the behaviour of VIDIOC_DQBUF. (And it must be documented as well, which is missing from the patch currently.)
-- Kind regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx -- 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
