RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1267584 - device.attributes.values likely to raise a KeyError
Summary: device.attributes.values likely to raise a KeyError
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: python-pyudev
Version: 7.2
Hardware: All
OS: Unspecified
unspecified
medium
Target Milestone: rc
: ---
Assignee: Jaroslav Škarvada
QA Contact: qe-baseos-daemons
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-09-30 13:23 UTC by mulhern
Modified: 2018-11-26 11:42 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 1265315
Environment:
Last Closed: 2018-11-26 11:42:59 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1283386 0 unspecified CLOSED udev_device_get_sysattr_list entry and udev_device_get_sysattr_value do not agree 2021-02-22 00:41:40 UTC

Internal Links: 1283386

Description mulhern 2015-09-30 13:23:39 UTC
+++ This bug was initially created as a clone of Bug #1265315 +++

Description of problem:

udev_device_get_sysattr_list_entry returns a list of attribute, value pairs.
The values are NULL, the attributes are said to be "available".

From the docs:
"""
 Retrieve the list of available sysattrs, with value being empty; This just return all available sysfs attributes for a particular device without reading their values. 
"""

I dunno what "available" is supposed to mean, but if these attributes are
further looked up by "udev_device_get_sysattr_value" they may have no value,
i.e., this method may return NULL. (This is not the same as having a value
like the empty string, which fits the specification just fine.)


Version-Release number of selected component (if applicable):

Any.

How reproducible:

Consistently, for a given device.

Steps to Reproduce:
1. Loop over all devices returned by udevadm.
2. List the attributes using udev_device_get_sysattr_list_entry().
3. Look up the listed attributes using udev_device_get_sysattr_value(). Observe that some of these attributes are NULL.

Actual results:

Some "available" attributes have no values. Generally speaking, these attributes seem to be symlinks to directories. It is not true that all attributes that correspond to symlinks to directories have no value, only some.

Expected results:

"Available" attributes should have a value or documentation should define clearly that "available" means fits some criteria which does not necessarily include actually having a value.

Additional info:

It is normal to think of attribute name/value pairs as being like keys in a map.
If they are not, that is, if a key can simultaneously be in the map and not be
in the map, that should get better documentation then by the single word "available".

Also, in documentation, the ';' is followed by a capital letter, should be lowercase.

--- Additional comment from mulhern on 2015-09-22 12:11:50 EDT ---

Output of new.c on my Fedora desktop. This is just an example, similar things
happen on my RHEL 7.2 box.

[mulhern@localhost reproducers]$ ./a.out
udev_list_entry_get_value()
Correct behavior, values are not obtained.
adr: (null)
path: (null)
subsystem: (null)
uevent: (null)
physical_node: (null)

udev_device_get_sysattr_value()
Incorrect behavior; physical_node should have a value.
adr: 0x00000000
path: \_SB_.PCI0.EHC2.HUBN
subsystem: acpi
uevent: 
physical_node: (null)

udev_device_get_sysattr_value(bogus)
correct behavior, value of non-existant attribute is null.
bogus: (null)

Should be like the first.
adr: (null)
path: (null)
subsystem: (null)
uevent: (null)
physical_node: (null)

--- Additional comment from mulhern on 2015-09-23 15:35 EDT ---



--- Additional comment from mulhern on 2015-09-23 17:03:36 EDT ---

The attached file printed SURPRISE a few times on both systems, indicating that some of the attributes with no value were not symlinked directories. What characteristics they do have I do not know.

-------------------------------------------------------------------------------

Since attributes inherits from Mapping, values method is defined by the Mapping class using in, like, "[s[key] for key in s]". Due to the libudev inconsistency described in #1265315, this is likely to raise an error when key is both in, in the second use of 'in', but not in in the first use of 'in'.

Comment 2 mulhern 2015-09-30 14:03:08 UTC
items(), iteritems(), and itervalues() all have the same problem, since they have the same dependency.

Basically, it would be good to know what the intention of libudev actually is before making a move on this bug.

But to avoid all these methods raising a KeyError, the only real option at this time is to have __getitem__() raise a KeyError when the item is not among the keys, but return None when it is among the keys but lookup fails. If that is the correct thing to do, then it is at least quite straightforward.

Comment 3 Kay Sievers 2015-10-02 12:31:34 UTC
> udev_device_get_sysattr_list_entry returns a list of attribute, value pairs.
> The values are NULL, the attributes are said to be "available".

It only returns a list of file names, like "ls". You cannot use the values in this list, they are not set.

> but if these attributes are further looked up by
> "udev_device_get_sysattr_value" they may have no value

The same way as "cat" of the file name returned by "ls" may result in
an error, no value, for whatever reason.

"physical_node" is a pointer to a device, not an attribute, trying to read it as an attribute value returns NULL. This this the expected behavior.

Note: Please be very careful by blindly reading all things available in /sys,
some attributes change the behavior of the system, just by reading the values.
/sys is not for poking around, without knowing what it does. Privileged
programs can cause harm here, just by opening and reading random files.

Comment 4 mulhern 2015-10-02 14:05:58 UTC
The other option is not to make Attributes a subclass of Mapping. That is an idea worth investigating, since the mapping is in general partial (see Comment #3).

Comment 5 mulhern 2015-10-02 16:09:25 UTC
This should be deferred until 7.3. It turns out that it is more of a design flaw than a simple fix, so it will require a change in the Attributes class, rather than just a simple two line change in a single method. At the same time, it seems that users of the library are not using it in a way that triggers the bug.
They are using it more like the underlying libudev library was intended to be used, apparently.

Comment 7 RHEL Program Management 2015-10-02 16:15:56 UTC
Development Management has reviewed and declined this request.
You may appeal this decision by reopening this request.

Comment 8 mulhern 2015-11-18 20:52:15 UTC
There is still a problem, because the two methods that this class depends on don't seem to have a very easy to discover relationship. So, it seems like this Attributes class should stop having any notion of keys at all until that gets worked out.

Comment 9 mulhern 2015-11-25 18:23:32 UTC
The relationship was easy to discover, but not really deep or meaningful, see related bug.

Comment 10 mulhern 2015-12-02 18:47:16 UTC
This is fixed in pyudev version 0.18.

Comment 11 mulhern 2015-12-03 19:28:28 UTC
I think my responsibility ends now that it is fixed upstream.

Comment 12 mulhern 2015-12-11 16:09:14 UTC
The fix results in some issues with Attributes.tostring() method, so I'm reassigning.

Comment 13 mulhern 2015-12-18 19:00:23 UTC
Fixed in version 0.18.1.

Comment 14 Fedora Update System 2015-12-22 15:57:44 UTC
python-pyudev-0.18.1-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-7eb77298d4

Comment 15 Fedora Update System 2015-12-23 00:09:58 UTC
python-pyudev-0.18.1-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-7eb77298d4

Comment 16 mulhern 2015-12-23 01:35:04 UTC
Whoops, an unintended effect of filling in some bodhi field.

Comment 17 Fedora Update System 2015-12-28 22:54:25 UTC
python-pyudev-0.18.1-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.