M0Rf30/android-udev-rules

The line takes no effect, ignoring.

b1tninja opened this issue · 6 comments

b1tninja commented

20190315

Reading rules file: /usr/lib/udev/rules.d/51-android.rules
/usr/lib/udev/rules.d/51-android.rules:73 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:210 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:221 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:303 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:351 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:353 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:355 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:514 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:517 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:520 The line takes no effect, ignoring.
/usr/lib/udev/rules.d/51-android.rules:523 The line takes no effect, ignoring.

ATTR{idProduct}=="7030"
ATTR{idProduct}=="0c93"
ATTR{idProduct}=="0c01"
ATTR{idProduct}=="618f"
ATTR{idProduct}=="2d66"
ATTR{idProduct}=="428c"
ATTR{idProduct}=="41db"
ATTR{idProduct}=="3137"
ATTR{idProduct}=="3138"
ATTR{idProduct}=="3149"
ATTR{idProduct}=="e14f"

are the lines in question, my guess is a , missing above

meganukebmp commented

This is mainly due to the weird setup here with the conditionals. As per specification udev rules are not meant to be dynamic. What's being done here with the conditionals and then if checks makes it more readable, but is inherently out of spec imo. This will be resolved if all rules include both idVendor and idProduct checks instead of checking once per vendor.

  • 😕1
jgottula commented

The lines that udev is complaining about look to me like no-op lines that were meant to effectively just serve as comments.

#	Sony Ericsson
ATTR{idVendor}!="0fce", GOTO="not_Sony_Ericsson"
ENV{adb_user}="yes"
#		Xperia X10 mini
ATTR{idProduct}=="3137"
ATTR{idProduct}=="2137", SYMLINK+="android_adb"
#		Xperia X10 mini pro
ATTR{idProduct}=="3138"
ATTR{idProduct}=="2138", SYMLINK+="android_adb"
#		Xperia X8
ATTR{idProduct}=="3149"
ATTR{idProduct}=="2149", SYMLINK+="android_adb"
#		Xperia X12
ATTR{idProduct}=="e14f"
ATTR{idProduct}=="614f", SYMLINK+="android_adb"

This snippet is a good example. What's happening here is that the author of this portion of the file has decided to document both the normal and debug mode USB product ID's for each of these devices. But then they only actually do anything to the latter type. (So, in effect, the function of the non-debug-mode PID lines, e.g. ATTR{idProduct}=="3137", is simply to say "3137 is the PID the X10 mini would have when NOT in debug mode", and to then explicitly show that nothing should be done in that case, as a contrast to PID 2137.)

Prior to systemd/udev v243, these lines would just silently be ignored. But in v243 (which, not coincidentally, was released on Sep 3 2019), a warning was added for cases where a line in a udev rules file has at least one syntactically valid match key, but then lacks any assignment/action keys that would do anything based on the matches.

So, as of udev v243 and later, this weird quirky pseudo-comment syntax used multiple times in this file is no longer a valid thing and generates annoying warnings.

There are a few ways the rules file could be modified to fix this and get rid of the warnings:

  1. (Best) Rather than having rule lines that match but then do nothing, just use the comment syntax to document what the non-debug USB PID is for each device instead. There is literally no point to a rule that matches a USB PID and then takes no actions based on that match.
  2. (Dumb) Make these lines technically correct by adding a GOTO after the matching part. (Probably to android_usb_rule_match, I would assume?) This approach works because doing a GOTO is technically an action. It's a pretty stupid fix though.
  3. (Dumb) Make these lines technically correct by having them define an unused LABEL after the matching part. This approach works because defining a LABEL is technically an action. As with the previous idea, this one is also stupid.

This is mainly due to the weird setup here with the conditionals. As per specification udev rules are not meant to be dynamic. What's being done here with the conditionals and then if checks makes it more readable, but is inherently out of spec imo. This will be resolved if all rules include both idVendor and idProduct checks instead of checking once per vendor.

Nah, the LABEL/GOTO stuff is technically fine, really; it arguably does make things more readable. The problem here has strictly to do with rules that match but don't act; those are certainly out of spec.

  • 👍3
ilf commented

There are a few ways the rules file could be modified to fix this and get rid of the warnings:

I would love to see this fixed. (It's pretty annoying in the systemd journal.)

ilf commented

Here are my error messages:

systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:89 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:226 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:237 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:319 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:367 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:369 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:371 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:540 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:543 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:546 The line takes no effect, ignoring.
systemd-udevd[PID]: /usr/lib/udev/rules.d/51-android.rules:549 The line takes no effect, ignoring.

This is my file: 51-android.rules

So the problematic lines are:

89: ATTR{idProduct}=="7030"
226: ATTR{idProduct}=="0c93"
237: ATTR{idProduct}=="0c01"
319: ATTR{idProduct}=="618f"
367: ATTR{idProduct}=="2d66"
369: ATTR{idProduct}=="428c"
371: ATTR{idProduct}=="41db"
540: ATTR{idProduct}=="3137"
543: ATTR{idProduct}=="3138"
546: ATTR{idProduct}=="3149"
549: ATTR{idProduct}=="e14f"
ilf commented

Closed with commit 8a21289 and released with 20200410. Thanks!

M0Rf30 commented

Thanks for your cooperation :D