[RFC PATCH] Initial libudev input-hotplug support

Julien Cristau jcristau at debian.org
Tue Oct 6 04:22:09 PDT 2009


On Mon, Oct  5, 2009 at 14:13:39 -0400, Adam Jackson wrote:

> On Tue, 2009-09-29 at 23:54 +0200, Julien Cristau wrote:
> 
> > - what's the recommended way to avoid races between the 'enumerate' and
> >   'monitor' parts?
> 
> I suspect the answer is "turn on monitor, then enumerate, then read plug
> events from the monitor", and no-op adding a device you've already
> added.
> 
In v2 I moved the call to udev_monitor_enable_receiving before
udev_enumerate_scan_devices, that should take care of this.

> > +	config_info = xalloc(strlen(syspath) + 6); /* "udev:" + \0 */
> > +	if (!config_info)
> > +		goto unwind;
> > +	sprintf(config_info, "udev:%s", syspath);
> 
> We have the moral equivalent of glibc's asprintf():
> 
>         config_info = Xprintf("udev:%s", syspath);
>         if (!config_info)
>                 goto unwind;
> 
> Which we should really use uniformly, it's a lot harder to get wrong.
> 
Cool, didn't know that one, thanks.

> > +static struct udev_monitor *udev_monitor;
> > +static OsTimerPtr udev_timer;
> > +
> > +static CARD32
> > +connect_timer(OsTimerPtr timer, CARD32 time, pointer arg) {
> > +	int fd;
> > +	struct udev_enumerate *enumerate;
> 
> Bad { placement, but...
> 
Fixed.

> > +int
> > +config_udev_init(void) {
> > +	struct udev *udev;
> > +
> > +	udev = udev_new();
> > +	udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
> > +	if (!udev_monitor)
> > +		return 0;
> > +	udev_timer = TimerSet(NULL, 0, 1, connect_timer, NULL);
> > +	if (!udev_timer)
> > +		return 0;
> > +	return 1;
> > +}
> 
> I don't think the timer should be needed for libudev.  All
> udev_monitor_new_from_netlink() does aside from malloc() is open a
> netlink socket.  If that fails you have a plethora of more serious
> problems already.
> 
udev_monitor_new_from_netlink is called on init, but I can't call
AddGeneralSocket from there, or actually add the input devices, because
config_init() is called too early for that.  Is there a way besides the
timer to schedule this to happen when the server is actually ready?

Cheers,
Julien


More information about the xorg-devel mailing list