[PATCH libinput 3/3] Change the logging system to be per-context

[Jonas Ådahl]

On Wed, Jun 18, 2014 at 08:39:01AM +1000, Peter Hutterer wrote:
> On Tue, Jun 17, 2014 at 11:20:00PM +0200, Jonas Ådahl wrote:
> > On Fri, Jun 13, 2014 at 12:48:33PM +1000, Peter Hutterer wrote:
> > > Rather than a single global logging function, make the logging dependent on
> > > the individual context. This way we won't stomp on each other's feet in the
> > > (admittedly unusual) case of having multiple libinput contexts.
> > > 
> > > The log handler and the log priority is now a part of the libinput interface.
> > > We can drop the various setters and getters, the caller owns the struct anyway
> > > so we don't need functions to give it those values.
> > > 
> > > The userdata argument to the log handler was dropped. The caller has a ref to
> > > the libinput context now, any userdata can be attached to that context
> > > instead.
> > > 
> > > There is no need for a default log function anymore. Any serious caller should
> > > hook into it anyway, those that don't care can just use NULL.
> > > 
> > > There is no default log priority anymore, a caller must set the desired
> > > priority in the interface.
> > > 
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > ---
> > > There's a side-effect to this that I'm not sure is intended. We don't copy
> > > the interface into libinput, we merely keep a reference. The caller is
> > > already able to change open_restricted/close_restricted at runtime, though
> > > we can't do this ourselves (it's const).
> > > 
> > > Given that, I figured we can leave the log handler and priority up to the
> > > caller as well then, switching at runtime. That's the main reason for
> > > dropping the set/get priority calls. If that side effect wasn't intended,
> > > then we'll have rework a few things. Jonas?
> > 
> > Not sure I like this change. The interface (function pointer struct) is
> > intended to really be an constant interface where the caller never
> > changes the function used. Of course it would be possible, given how its
> > implemnted, but it was not intended.
> 
> so should we copy the struct then? or trust callers to not do anything
> untoward?
> 
> > The purpose of the struct was to provide an interface with the
> > functionality that libinput would require to have to function without
> > having to be root, and it doesn't feel logging function fits this
> > purpose. It was already a set/get, wouldn't it fit better to just make
> > them per context, while keeping the interface struct minimal?
> 
> yeah, fair enough. I arrived at this solution at a bit of a roundabout way
> since I wanted to make the current udev_create work without changes and then
> failed anyway. with the create_context function it's possible now to do the
> following:
> 
>     li = libinput_udev_create_context();
>     libinput_set_log_handler(li, ...);
>     libinput_set_log_priority(li, ...);
>     libinput_udev_set_set(li, seat);
> 
> That's what you're proposing, right?

Yes, something like that. We could also possibly do what I think we
discussed a while ago and default to suspended, i.e. _create_for_seat(),
_set_(), _enable(), but either way really. The problem with _set_seat()
though I guess is that it makes it look like we can actually switch seat
of a context, which I don't know if we should support.

Jonas

> 
> Cheers,
>    Peter