Bug 13361

Summary: CONFIG_HAL is enabled by default (if hal subsystem is available at build time) but is broken
Product: xorg Reporter: Paulo César Pereira de Andrade <pcpa>
Component: Input/otherAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED WONTFIX QA Contact: Xorg Project Team <xorg-team>
Severity: blocker    
Priority: high CC: ademar
Version: git   
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
This patch is broken, not finished, don't use it. none

Description Paulo César Pereira de Andrade 2007-11-22 17:25:25 UTC
First I will start with a clear bug in config/hal.c:

In function get_prop_string_array() it is allocating the proper number of
bytes, or one less if adding an ending "," is considered correct, like it
is doing. Also, it appears to expect strcpy to return a pointer to the end
of the string, what is not true. I.e. if input.xkb.options = option1, option2
it will generate the string ",,"
It is also handling input.xkb.layout as a single string. But multiple layouts
can be specified, but if the user specifies the string in the proper format,
i.e. "layout1,layout2" it should be parsed properly.
But the major problem is that the X Server uses whatever is specified in
xorg.conf, and config/hal.c will add again the input device specified in hal
(unless the function doesn't like the information it collects and doesn't call
NewInputDeviceRequest()), and it can end with two input modules (possibly
different drivers reading from different file descriptors) handling the same
device, and weird results will happen.

It took me sometime to understand most of the "internals" of what is going on;
I did not check what other distros are doing, but before compiling Mandriva's
cooker (i.e. bleeding edge for adventurous users) with --disable-config-hal
I would like to give some feedback about the problem, so that I can either
enter a consensus about resolving the problem, or decide that it is better
to wait until the problems are resolved. I plan to come back to these issues
soon or later, but it is kind of out of order with the "fixes" I plan to do
to Mandriva's X Server, i.e. I have an almost fully functional implementation
compiled with -fvisibility=hidden (i.e. explicitly make symbols available
using _X_EXPORT) but I want to have minimal chances of failure before making
it default to Mandriva Cooker users. After that I plan some redesign on the
loader and modules API...


Cut & paste of my message to xorg-list for some more details:
-%<-
  Hi,

  Recently we updated Mandriva cooker to server-1.4-branch, so, this
in is email I should give some feedback and suggestions.

  First, maybe it should not be in what one would expect, based on the
tag name, as a "bug fixes only" branch :-) But the details:

  I had some reports of people having all kinds of weird problems,
while I was not experiencing any of those. But the reason is that I
use a more customized setup, and did not have the x11-driver-input-evdev
package installed, and this single fact would basically disable the
config/hal.c code.

  But the major problem I see is that, if there is an "InputDevice" in
xorg.conf, the X Server will use it, and, the hal code will also work,
and weird things will happen, like 2 instances of an input device. At
best one would have the problem I was having that is, pressing a mouse
button would always generate a double click :-)

  I am attempting to make a patch, fix the problem without doing the
"simple" fix of disabling hal and waiting to see how it evolves..., so
I would like to know if others used some different approach, or if
there is work in progress.

  One of the major problems to make a "temporary" patch, until xorg.conf
becomes obsolete would be merge xorg.conf options and hal properties,
giving preference to xorg.conf. But the major problem is that there isn't
a mapping from hal devices to xorg.conf devices. A simple guess of
keyboard/mouse can be done, but I am not sure about other devices, or
any "xorg.conf" device that uses a driver other than "kbd" or "mouse".
I considered to do something like:
  hal:         input.x11_identifier = "Mouse1"
  xorg.conf:   Option "hal_udi"
"/org/freedesktop/Hal/devices/platform_i8042_i8042_AUX_port_logicaldev_input"
  This way, code in the X Server could know that both specify the same
hardware, and don't try to have two instances of the same (or different)
input module(s) managing the same device. But this would require some
"guessing" and asking the user for confirmation isn't an option.

  One could ignore xorg.conf "InputDevice" section (what I believe should
not be done now), but then, what to do if hal is misconfigured or
something like the proper x11_driver ins't installed? Start the X Server
without a keyboad input module?

  What would be the proper method of configuring hal? Using
hal_get_property, hal_set_property, and similar tools? i.e. using the
example above, when I run something like:
$ hal-get-property --udi
/org/freedesktop/Hal/devices/platform_i8042_i8042_AUX_port_logicaldev_input
--key input.x11_driver
I get the response:
mouse
  But this is because I am working on a patch that tries to merge
xorg.conf options with hal.
  Another option, could be to use the similar of
"hal-device --remove <udi>"
if the device exists in xorg.conf, but this just comes back to the fact
that there isn't a mapping from hal devices to xorg.conf devices.

  Any comments?

Thanks,
Paulo
-%<-
Comment 1 Paulo César Pereira de Andrade 2007-12-01 04:37:50 UTC
Created attachment 12895 [details] [review]
This patch is broken, not finished, don't use it. 

Only attached for documentation and to use it for information on how to track other data structures from the "root ones".
Comment 2 Peter Hutterer 2008-02-27 15:30:18 UTC
(In reply to comment #0)
> First I will start with a clear bug in config/hal.c:
> 
> In function get_prop_string_array() it is allocating the proper number of
> bytes, or one less if adding an ending "," is considered correct, like it
> is doing. Also, it appears to expect strcpy to return a pointer to the end
> of the string, what is not true. I.e. if input.xkb.options = option1, option2
> it will generate the string ",,"

FWIW this part was fixed in a8e27a108abeba73b2888da4e0604008f4b02045
Comment 3 Peter Hutterer 2008-04-30 22:31:53 UTC
sorry Paulo, I'm losing track about the actual problem in this bug.

(In reply to comment #0)
> But the major problem is that the X Server uses whatever is specified in
> xorg.conf, and config/hal.c will add again the input device specified in hal
> (unless the function doesn't like the information it collects and doesn't call
> NewInputDeviceRequest()), and it can end with two input modules (possibly
> different drivers reading from different file descriptors) handling the same
> device, and weird results will happen.

the server can't know which device has been added already. reason being that if a section exists for /dev/input/mouse1, the server doesn't know this is also /dev/input/event3.

>   But the major problem I see is that, if there is an "InputDevice" in
> xorg.conf, the X Server will use it, and, the hal code will also work,
> and weird things will happen, like 2 instances of an input device. At
> best one would have the problem I was having that is, pressing a mouse
> button would always generate a double click :-)

evdev does a EVIOCGRAB on the device, so this shouldn't actually happen. I tried it with a current git version, and it doesn't. Can you reproduce this problem?
 
>   One of the major problems to make a "temporary" patch, until xorg.conf
> becomes obsolete would be merge xorg.conf options and hal properties,
> giving preference to xorg.conf. But the major problem is that there isn't
> a mapping from hal devices to xorg.conf devices. A simple guess of
> keyboard/mouse can be done, but I am not sure about other devices, or
> any "xorg.conf" device that uses a driver other than "kbd" or "mouse".

I believe the correct solution is to make evdev aware of these devices.
Comment 4 Paulo César Pereira de Andrade 2008-04-30 23:02:44 UTC
(In reply to comment #3)
> sorry Paulo, I'm losing track about the actual problem in this bug.

  I did not look at this stuff since I wrote the bugreport, and
currently Mandriva X Server is being compiled without dbus/hal
support. But I should work/test it again soon, as I hope to be able
to ship a newer X Server, possibly not even needing a configuration
file anymore in next Mandriva release, but I think I will watch Fedora
and *Debian go first, and if they don't sink I follow :-)

> (In reply to comment #0)
> > But the major problem is that the X Server uses whatever is specified in
> > xorg.conf, and config/hal.c will add again the input device specified in hal
> > (unless the function doesn't like the information it collects and doesn't call
> > NewInputDeviceRequest()), and it can end with two input modules (possibly
> > different drivers reading from different file descriptors) handling the same
> > device, and weird results will happen.
> 
> the server can't know which device has been added already. reason being that if
> a section exists for /dev/input/mouse1, the server doesn't know this is also
> /dev/input/event3.

  That is the main reason of the bug report :-), there must be only
one "entity" managing the devices. In that case it should be hal, as
X Server code (and code that handles xorg.conf) don't play well with
hal because there is no mapping about who is managing what.

> >   But the major problem I see is that, if there is an "InputDevice" in
> > xorg.conf, the X Server will use it, and, the hal code will also work,
> > and weird things will happen, like 2 instances of an input device. At
> > best one would have the problem I was having that is, pressing a mouse
> > button would always generate a double click :-)
> 
> evdev does a EVIOCGRAB on the device, so this shouldn't actually happen. I
> tried it with a current git version, and it doesn't. Can you reproduce this
> problem?

  The evdev driver seens a bit more stable now, but at the time of
the bugreport, it just could not replace kdb and mouse, maybe kdb
and mouse drivers could be changed to know about EVIOCGRAB also...

> >   One of the major problems to make a "temporary" patch, until xorg.conf
> > becomes obsolete would be merge xorg.conf options and hal properties,
> > giving preference to xorg.conf. But the major problem is that there isn't
> > a mapping from hal devices to xorg.conf devices. A simple guess of
> > keyboard/mouse can be done, but I am not sure about other devices, or
> > any "xorg.conf" device that uses a driver other than "kbd" or "mouse".
> 
> I believe the correct solution is to make evdev aware of these devices.

  Even tough I work for a Linux company, I am also a user of other
OSS operating systems, and I don't feel confortable with evdev as
it is Linux specific. But hopefuly, for my Mandriva packages at least,
this should not be a problem anymore soon.

Comment 5 Peter Hutterer 2008-04-30 23:14:55 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > the server can't know which device has been added already. reason being that if
> > a section exists for /dev/input/mouse1, the server doesn't know this is also
> > /dev/input/event3.
> 
>   That is the main reason of the bug report :-), there must be only
> one "entity" managing the devices. In that case it should be hal, as
> X Server code (and code that handles xorg.conf) don't play well with
> hal because there is no mapping about who is managing what.

I'll take this as a reason to close this bug as WONTFIX. It isn't really a server problem, we just add what we get told. Anything more fancy needs to be in hal (or the other entity).

> > >   But the major problem I see is that, if there is an "InputDevice" in
> > > xorg.conf, the X Server will use it, and, the hal code will also work,
> > > and weird things will happen, like 2 instances of an input device. At
> > > best one would have the problem I was having that is, pressing a mouse
> > > button would always generate a double click :-)
> > 
> > evdev does a EVIOCGRAB on the device, so this shouldn't actually happen. I
> > tried it with a current git version, and it doesn't. Can you reproduce this
> > problem?
> 
>   The evdev driver seens a bit more stable now, but at the time of
> the bugreport, it just could not replace kdb and mouse, maybe kdb
> and mouse drivers could be changed to know about EVIOCGRAB also...

AFAIK EVIOCGRAB only works on evdev devices. mouse and kbd don't use evdev kernel devices, so that can't happen.

> > >   One of the major problems to make a "temporary" patch, until xorg.conf
> > > becomes obsolete would be merge xorg.conf options and hal properties,
> > > giving preference to xorg.conf. But the major problem is that there isn't
> > > a mapping from hal devices to xorg.conf devices. A simple guess of
> > > keyboard/mouse can be done, but I am not sure about other devices, or
> > > any "xorg.conf" device that uses a driver other than "kbd" or "mouse".
> > 
> > I believe the correct solution is to make evdev aware of these devices.
> 
>   Even tough I work for a Linux company, I am also a user of other
> OSS operating systems, and I don't feel confortable with evdev as
> it is Linux specific. But hopefuly, for my Mandriva packages at least,
> this should not be a problem anymore soon.

neither kbd nor mouse support hotplugging. until they are updated, evdev is the only way if you want input device hotplugging.




Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.