From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57647 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753230Ab1KBKD4 (ORCPT ); Wed, 2 Nov 2011 06:03:56 -0400 Message-ID: <4EB115A0.9030008@redhat.com> Date: Wed, 02 Nov 2011 11:04:16 +0100 From: Hans de Goede MIME-Version: 1.0 To: Hans Verkuil CC: Linux Media Mailing List Subject: Re: [PATCH 5/6] v4l2-event: Add v4l2_subscribed_event_ops Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: 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