[PATCH libinput 6/6] doc: put some extra warning in for libinput_event_destroy()

Jonas Ådahl jadahl at gmail.com
Wed Dec 10 17:19:30 PST 2014


On Thu, Dec 11, 2014 at 09:07:12AM +1000, Peter Hutterer wrote:
> On Wed, Dec 10, 2014 at 06:01:13PM +0800, Jonas Ådahl wrote:
> > Hi,
> > 
> > Comment inline.
> > 
> > On Wed, Dec 10, 2014 at 10:34:04AM +1000, Peter Hutterer wrote:
> > > Unlike all other structs, events aren't refcounted and will get destroyed
> > > immediately.
> > > 
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > ---
> > >  src/libinput.h | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/libinput.h b/src/libinput.h
> > > index 7e5d93c..f7cb169 100644
> > > --- a/src/libinput.h
> > > +++ b/src/libinput.h
> > > @@ -335,6 +335,9 @@ struct libinput_seat;
> > >   *
> > >   * The base event type. Use libinput_event_get_pointer_event() or similar to
> > >   * get the actual event type.
> > > + *
> > > + * @warning Unlike other structs events are considered transient and
> > > + * <b>not</b> refcounted.
> > >   */
> > >  struct libinput_event;
> > >  
> > > @@ -382,7 +385,12 @@ struct libinput_event_touch;
> > >  /**
> > >   * @ingroup event
> > >   *
> > > - * Destroy the event.
> > > + * Destroy the event, freeing all associated data. Data obtained from this
> > > + * event must be considered invalid after this call.
> > 
> > Hmm. Is this really correct? The validity of some data, for example the
> > coordnates of an absolute motion event, is not related to the lifetime
> > of the event, but should rather be considered invalid after some (very
> > short) time, so saying that an event invalidates all data of an event
> > doesn't seem to make sense to me. What it does invalidate would be any
> > data accessed via a pointer, so maybe that is what should be written
> > instead?
> 
> right, in this case "data" in the context of the C language, so this
> paraphaph was to avoid invalid memory access by callers. How about
> "resources" instead of data.
> 
> + * Destroy the event, freeing all associated resources. Resources obtained from this
> + * event must be considered invalid after this call.
> 
> That should be enough, the varous devices/seat/etc. structs all have notes
> that their lifetime is tied to the event and need to be ref'd to extend the
> lifetime.

That makes it indeed clear enough IMO. Reviewed-by: Jonas Ådahl
<jadahl at gmail.com> for updated this and the other as well.


Jonas

> 
> Cheers,
>    Peter
> 
> 
> > > + *
> > > + * @warning Unlike other structs events are considered transient and
> > > + * <b>not</b> refcounted. Calling libinput_event_destroy() <b>will</b>
> > > + * destroy the event.
> > >   *
> > >   * @param event An event retrieved by libinput_get_event().
> > >   */
> > > -- 
> > > 2.1.0
> > 
> > Jonas


More information about the wayland-devel mailing list