Re: [PATCH 5/6] v4l2-event: Add v4l2_subscribed_event_ops

[Hans de Goede <hdegoede@redhat.com>]

Hi,

hverkuil wrote:
> > > +	if (sev->ops && sev->ops->add) {
> > > +		int ret = sev->ops->add(sev);
> > > +		if (ret) {
> > > +			sev->ops = NULL;
> > > +			v4l2_event_unsubscribe(fh, sub);
> > > +			return ret;
> > > +		}
>
> The problem here is that the event is basically available for use after the
> spin_unlock_irqrestore(), but before the sev->ops->add() call. In the past I
> just 'knew' that the event would never be generated by the control framework
> until after v4l2_ctrl_add_event was called, but this should be formalized now
> that these ops are added.
>
> I see two options:
>
> 1) Have some method to mark the sev as being 'invalid' so functions sending
> events can skip it (needed as well for your patch 4/6).
>
> 2) Document that drivers should never be able to send an event until after
> the add() callback has succeeded.
>
> I am leaning towards option 1 myself.
>
> How about leaving sev->elems at 0 until after the add() op succeeds?
>
> That's easy to test against and easy to implement.

I agree that option 1 is the best and that leaving sev->elems 0 is a good
way to do this. This will be in my next revision of this patch set.

Thanks & Regards,

Hans