[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