Thursday, February 12, 2009

My First Patch Submission

21:00 - Hmm, that's odd, the device manager on my Ubuntu laptop won't open up. It crashes. Very strange.

21:05 - Wait a second, that's not just a crash, that's a Python stack-trace! I know Python, I bet I could figure out the problem here. (And then everyone will love me and my name will live forever etc etc. Really I'm just in it for the girls.)

21:15 - Hey, it looks like hal-device-manager only consists of two Python scripts! This should only take five minutes.

00:10 - Wow that took a long time. But I've finally found the bug (see below) and I'm ready to submit it.

00:15 - Uh, where do I submit it?

00:30 - OK, so it looks like Ubuntu are getting the code off of Debian, who are getting it off FreeDesktop.org. Before I get in touch, I'd better check that the FreeDesktop folks haven't fixed it in the latest version of their code.

00:40 - Uh, where in tarnation is the code for this app? I can't find it in the git repository anywhere? Better google for it.

00:45 - Whadda ya mean, obsolete? NOOOOOOOOOOOOO!!!!!!!!!!!!!!!!!!! *sob*

For anyone who is the least bit interested in this sort of thing, the bug was actually kinda cool. What the code attempts to do is:

1) generate a list of devices (each of which has a predefined "parent" attribute)
2) for each device, create a list of that device's children (thus defining a device hierarchy) and ensure that the children's "parent" attribute points nicely at the parent
3) recursively set some of the children's attributes based on the parent's attribute

This works fine... except when there is more than one device with exactly the same name, and where those twin devices have a child. In this case, step 2 will result in the third device being registered as a child of both twins. From the child's point of view, the second twin will be designated as its parent.

When step 3 happens, the first twin is initialised. Then its child is initialised. But to do this, the child needs to check some of its parent's attributes. And who is the parent? The second twin, which hasn't yet been initialised. So the child can't get the info it needs and thusly throws a tantrum.

The patch is to stick a strategic "break" statement into the code:
--- DeviceManager.py.old 2009-02-12 23:15:48.000000000 +0000
+++ DeviceManager.py 2009-02-12 23:16:44.000000000 +0000
@@ -286,6 +286,7 @@
if p.device_name==parent_name:
device.parent_device = p
p.children.append(device)
+ break
if device!=virtual_root and device.parent_device==virtual_root:
virtual_root.children.append(device)
if device==virtual_root:


I suspect this situation won't come up very much, possibly only on those systems that (like me) have a Broadcom BCM4401-B0 100Base-TX ethernet controller.

Read the full post