Reading Android code
I'm trying to like Android code but failing. I want to share today's challenge. Here is top of tree Android EventHub code. When a device is removed, it does close_device which does things like:
You see the memmove-s and think, hmm, odd, wonder what they're trying to do. You look in open_device:
and you see what looks like an attempt to map a device ID to a collection of data associated with it, most importantly its fd. It looks like a very odd way of doing it, but hey, if it works , then great, we can just ignore above oddness and go on with our lives. Problem is, that this stuff doesn't work. Even for simple things! When a device is removed, and then plugged back in, Android userspace fails in exciting ways.
Lets work an example. The first device that had been added has devid 0, and when it is removed, the other devices are pulled back with the memmove of mFDs and mDevicesById[0].device entry is set to null. But then those devices that were pulled back haven't had their IDs changed, so the:
results in the wrong fd being used for everything else since id n was memmoved to n-1 and the id_to_index doesn't account for that. The new device then picks up devid 0 (since that was the first mDeviceById[].device that was null) and then everything fails since mFDs[mFDcount] and count is 8 so it fails. fails. fails. :-( Sad. Okay, going to lunch now.
for(i = 1; i < index =" (device-">id&ID_MASK);
mDevicesById[index].device = NULL;
close(mFDs[i].fd);
int count = mFDCount - i - 1;
memmove(mDevices + i, mDevices + i + 1, sizeof(mDevices[0]) * count);
memmove(mFDs + i, mFDs + i + 1, sizeof(mFDs[0]) * count);
mFDCount--;
You see the memmove-s and think, hmm, odd, wonder what they're trying to do. You look in open_device:
int devid = 0;
while (devid < mNumDevicesById) {
if (mDevicesById[devid].device == NULL) {
break;
}
devid++;
}
mFDs[mFDCount].fd = fd;
mFDs[mFDCount].events = POLLIN;
and you see what looks like an attempt to map a device ID to a collection of data associated with it, most importantly its fd. It looks like a very odd way of doing it, but hey, if it works , then great, we can just ignore above oddness and go on with our lives. Problem is, that this stuff doesn't work. Even for simple things! When a device is removed, and then plugged back in, Android userspace fails in exciting ways.
Lets work an example. The first device that had been added has devid 0, and when it is removed, the other devices are pulled back with the memmove of mFDs and mDevicesById[0].device entry is set to null. But then those devices that were pulled back haven't had their IDs changed, so the:
#define id_to_index(id) ((id& ID_MASK)+1)
if (ioctl(mFDs[id_to_index(device->id)].fd,
results in the wrong fd being used for everything else since id n was memmoved to n-1 and the id_to_index doesn't account for that. The new device then picks up devid 0 (since that was the first mDeviceById[].device that was null) and then everything fails since mFDs[mFDcount] and count is 8 so it fails. fails. fails. :-( Sad. Okay, going to lunch now.
3 Comments:
The code is not displaying properly
Yeah you found a bug in the software, very impressive, indeed.
Hi greaat reading your blog
Post a Comment
<< Home