Bug 88410 - udev: * glob works wrongly in 60-keyboard.hwdb
Summary: udev: * glob works wrongly in 60-keyboard.hwdb
Status: NEW
Alias: None
Product: systemd
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Kay Sievers
QA Contact: systemd-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-14 16:59 UTC by Maxim Mikityanskiy
Modified: 2015-04-14 13:22 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Maxim Mikityanskiy 2015-01-14 16:59:16 UTC
I use MSI Wind U100 laptop. My DMI is:

dmi:bvnAmericanMegatrendsInc.:bvr4.6.3:bd12/01/2009:svnMICRO-STARINTERNATIONALCO.,LTD:pnU90/U100:pvrVer.001:rvnMICRO-STARINTERNATIONALCO.,LTD:rnU90/U100:rvrVer.001:cvnMICRO-STARINTERNATIONALCO.,LTD:ct10:cvrVer.001:

There are some rules that should match my laptop in /lib/udev/hwdb.d/60-keyboard.hwdb:


keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMicro-Star*:pn*
 KEYBOARD_KEY_a0=mute                                   # Fn+F9
 KEYBOARD_KEY_ae=volumedown                             # Fn+F7
 KEYBOARD_KEY_b0=volumeup                               # Fn+F8
 KEYBOARD_KEY_b2=www                                    # e button
 KEYBOARD_KEY_df=sleep                                  # Fn+F12
 KEYBOARD_KEY_e2=bluetooth                              # satellite dish2
 KEYBOARD_KEY_e4=f21                                    # Fn+F3 Touchpad disable
 KEYBOARD_KEY_ec=email                                  # envelope button
 KEYBOARD_KEY_ee=camera                                 # Fn+F6 camera disable
 KEYBOARD_KEY_f6=wlan                                   # satellite dish1
 KEYBOARD_KEY_f7=brightnessdown                         # Fn+F4
 KEYBOARD_KEY_f8=brightnessup                           # Fn+F5
 KEYBOARD_KEY_f9=search

#
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U-100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*N033:*
 KEYBOARD_KEY_f7=reserved
 KEYBOARD_KEY_f8=reserved

#
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*
 KEYBOARD_KEY_e4=reserved


The problem is that string "keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" for some reason does not match my laptop. I tried "keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:pvr*", and it worked. After some tries I found out that if "pn" is followed by "*", match will not work. So:

doesn't work: keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*
doesn't work: keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U90/U100:pvr*
works fine: keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:pvr*
works fine: keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90*:pvr*
doesn't work: keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*90*:pvr*

Obviously this is a bug, * should work like expected. I'm on systemd 218, Gentoo ~x86.
Comment 1 Zbigniew Jedrzejewski-Szmek 2015-01-17 23:00:18 UTC
It's a question of ordering. Since both the generic MICRO-STAR and the more specific U100/U90/etc patterns match, which ever gets assigned last wins. I guess that this is by design, since the documentation does not say anything about the order. Let's see what Kay thinks.

@Kay: ^^^
Comment 2 Maxim Mikityanskiy 2015-01-17 23:15:26 UTC
(In reply to Zbigniew Jedrzejewski-Szmek from comment #1)
> It's a question of ordering. Since both the generic MICRO-STAR and the more
> specific U100/U90/etc patterns match, which ever gets assigned last wins. I
> guess that this is by design, since the documentation does not say anything
> about the order. Let's see what Kay thinks.
> 
> @Kay: ^^^

Indeed, I tried to remove that lines from generic rule:

 KEYBOARD_KEY_f7=brightnessdown                         # Fn+F4
 KEYBOARD_KEY_f8=brightnessup                           # Fn+F5

And udevadm info /dev/input/event5 shows f7 and f8 as reserved, so *U100* rule works. So asterisk is not a bug. MSI rules rely on order of applying, but it is unspecified.

I think the order in which rules are applied has to be specified. Old 95-keymap.rules were applied from top to bottom in order. New 60-keyboard.hwdb is simply generated from 95-keymap.rules. But the ordering is broken now. Maybe, MSI is not the only case where it is important.
Comment 3 Kay Sievers 2015-01-18 18:57:58 UTC
(In reply to Zbigniew Jedrzejewski-Szmek from comment #1)
> It's a question of ordering. Since both the generic MICRO-STAR and the more
> specific U100/U90/etc patterns match, which ever gets assigned last wins. I
> guess that this is by design, since the documentation does not say anything
> about the order. Let's see what Kay thinks.

Right. All matches are applied in order to allow merging of multiple variables from different sections/files. If the specified key names are identical, the later ones overwrite the earlier ones. That is the expected behavior.
Comment 4 Zbigniew Jedrzejewski-Szmek 2015-01-18 18:59:18 UTC
> the later ones overwrite the earlier ones
"later" in what sense: ordered lower in the file?
Comment 5 Kay Sievers 2015-01-18 19:13:14 UTC
(In reply to Zbigniew Jedrzejewski-Szmek from comment #4)
> > the later ones overwrite the earlier ones
> "later" in what sense: ordered lower in the file?

It's just read as a stream of records. All file names are sorted, the file contents are applied record -by-record from top to bottom.
Comment 6 Maxim Mikityanskiy 2015-01-18 19:21:02 UTC
(In reply to Kay Sievers from comment #5)
> It's just read as a stream of records. All file names are sorted, the file
> contents are applied record -by-record from top to bottom.

This seem to do not work. When two rules are present:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*

the first one wins independently of order in file.

When these two rules are present:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*

the second one always wins independently of order in file.

It looks like the order in file is ignored, only the names are important.
Comment 7 Zbigniew Jedrzejewski-Szmek 2015-01-18 19:40:20 UTC
> All file names are sorted, the file contents are applied record -by-record from top to bottom.
Kay, I think you're thiking about rules files. This is about the hwdb.

I added some debugging statements to figure this out.

fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
hwdb_add_property: KEYBOARD_KEY_f7 → reserved
hwdb_add_property: KEYBOARD_KEY_f8 → reserved
fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*" → yes
hwdb_add_property: KEYBOARD_KEY_a0 → mute
hwdb_add_property: KEYBOARD_KEY_ae → volumedown
hwdb_add_property: KEYBOARD_KEY_b0 → volumeup
hwdb_add_property: KEYBOARD_KEY_b2 → www
hwdb_add_property: KEYBOARD_KEY_df → sleep
hwdb_add_property: KEYBOARD_KEY_e2 → bluetooth
hwdb_add_property: KEYBOARD_KEY_e4 → f21
hwdb_add_property: KEYBOARD_KEY_ec → email
hwdb_add_property: KEYBOARD_KEY_ee → camera
hwdb_add_property: KEYBOARD_KEY_f6 → wlan
hwdb_add_property: KEYBOARD_KEY_f7 → brightnessdown
hwdb_add_property: KEYBOARD_KEY_f8 → brightnessup
hwdb_add_property: KEYBOARD_KEY_f9 → search
fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*" → yes
hwdb_add_property: KEYBOARD_KEY_e4 → reserved

while hwdb file has:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMicro-Star*:pn*
 KEYBOARD_KEY_a0=mute                                   # Fn+F9
 KEYBOARD_KEY_ae=volumedown                             # Fn+F7
 KEYBOARD_KEY_b0=volumeup                               # Fn+F8
 KEYBOARD_KEY_b2=www                                    # e button
 KEYBOARD_KEY_df=sleep                                  # Fn+F12
 KEYBOARD_KEY_e2=bluetooth                              # satellite dish2
 KEYBOARD_KEY_e4=f21                                    # Fn+F3 Touchpad disable
 KEYBOARD_KEY_ec=email                                  # envelope button
 KEYBOARD_KEY_ee=camera                                 # Fn+F6 camera disable
 KEYBOARD_KEY_f6=wlan                                   # satellite dish1
 KEYBOARD_KEY_f7=brightnessdown                         # Fn+F4
 KEYBOARD_KEY_f8=brightnessup                           # Fn+F5
 KEYBOARD_KEY_f9=search

# some MSI models generate ACPI/input events on the LNXVIDEO input devices,
# plus some extra synthesized ones on atkbd as an echo of actually changing the
# brightness; so ignore those atkbd ones, to avoid loops
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U-100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*N033:*
 KEYBOARD_KEY_f7=reserved
 KEYBOARD_KEY_f8=reserved

# MSI Wind U90/U100 generates separate touchpad on/off keycodes so ignore touchpad toggle keycode
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*
 KEYBOARD_KEY_e4=reserved
Comment 8 Kay Sievers 2015-01-18 20:00:54 UTC
(In reply to Zbigniew Jedrzejewski-Szmek from comment #7)
> > All file names are sorted, the file contents are applied record -by-record from top to bottom.
> Kay, I think you're thiking about rules files. This is about the hwdb.

No, all fine, this was about hwdb.

> I added some debugging statements to figure this out.
> 
> fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
> hwdb_add_property: KEYBOARD_KEY_f7 → reserved
> hwdb_add_property: KEYBOARD_KEY_f8 → reserved

These are the keys which are specified later in the source hwdb files, but
they are applied first here. These later ones are supposed to overwrite the
earlier ones. The "reserved" should win here.

This is a bug in the hwdb code. Maybe it things changed recently regarding
this logic?
Comment 9 Zbigniew Jedrzejewski-Szmek 2015-01-18 20:14:59 UTC
Those entries are put in the trie. How is the ordering supposed to be preserved?
Comment 10 Kay Sievers 2015-01-18 20:15:36 UTC
(In reply to Kay Sievers from comment #8)
> (In reply to Zbigniew Jedrzejewski-Szmek from comment #7)
> > > All file names are sorted, the file contents are applied record -by-record from top to bottom.
> > Kay, I think you're thiking about rules files. This is about the hwdb.
> 
> No, all fine, this was about hwdb.
> 
> > I added some debugging statements to figure this out.
> > 
> > fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
> > hwdb_add_property: KEYBOARD_KEY_f7 → reserved
> > hwdb_add_property: KEYBOARD_KEY_f8 → reserved
> 
> These are the keys which are specified later in the source hwdb files, but
> they are applied first here. These later ones are supposed to overwrite the
> earlier ones. The "reserved" should win here.
> 
> This is a bug in the hwdb code. Maybe it things changed recently regarding
> this logic?

Oh, we have multiple records with _different_ match strings which get merged.
I forgot about this detail there.

With different match strings, it should not be about the order in the
file, but about the length of the match string. The data is put in a
patricia trie and the order of individual records should _not_ matter for _different_ match strings.

The intended behavior is that records with longer match strings overwrite
shorter match strings. That means, things which are stored "deeper" in the
tree are more specific/"better" matches than the broader ones stored closer
to the root. And the more specific ones should overwrite the broader ones.

Seem like this logic should be fixed.


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.