Comment 4 for bug 219424

Revision history for this message
Alfred Chen (alfredchen) wrote :

Based on your gdb trace log and the code I google from
http://launchpadlibrarian.net/13569808/ps3-xorg-gdb-session-chooseVideoDriver
Here is my analysis

+char*
+chooseVideoDriver(void)
+{
...
+ /* Find the primary device, and get some information about it. */
+ if (xf86PciVideoInfo) {
+ for (pciptr = xf86PciVideoInfo; (info = *pciptr); pciptr++) {
+ if (xf86IsPrimaryPci(info)) {
+ break;
+ }
+ }
+ if (!info) {
+ ErrorF("Primary device is not PCI\n");
+ }
+ } else {
+ ErrorF("xf86PciVideoInfo is not set\n");
+ }
+
+ if (!info) {
+ ErrorF("Could not get primary PCI info\n");
+ goto end;
+ }
+
+ idsdir = opendir("/usr/share/xserver-xorg/pci");
...
+ /* TODO Handle multiple drivers claiming to support the same PCI ID */
+ if (matches[0]) {
+ chosen_driver = matches[0];
+ } else {
+ #if defined __i386__ || defined __amd64__ || defined __hurd__
+ chosen_driver = "vesa";
+ #elif defined __alpha__
+ chosen_driver = "vga";
+ #elif defined __sparc__
+ chosen_driver = "sunffb";
+ #else
+ chosen_driver = "fbdev";
+ #endif
+ }
+
+ xf86Msg(X_DEFAULT, "Matched %s for the autoconfigured driver\n",
chosen_driver);
+
+ end:
+ i = 0;
+ while (matches[i]) {
+ if (matches[i] != chosen_driver) {
+ xfree(matches[i]);
+ }
+ i++;
+ }
+ xfree(line);
+ closedir(idsdir);
+
+ return chosen_driver;
+}

I think the root cause is xf86PciVideoInfo is not set in PS3, which
cause the 'info' is NULL and 'goto end' is called.
Above the end label, you can see there is platform default
chosen_driver selection, we are missing these code when 'goto end'
called.
This also casue the crash problem you descript at
https://bugs.launchpad.net/ubuntu-ps3-port/+bug/217647.
In the 'xf86PciVideoInfo is not set' situation, 'closedir(idsdir);' is
called before its initialized, so the crash comes. PS, I agreed with
Bryce's patch.

Solutions I can think about,
1. Find out why xf86PciVideoInfo is not set in PS3, is it normal or
not. AIK, the vedio card device provided in ps3 linux may not be a
real device. Some info may missing.
2. Move the 'end' label above the /* TODO xxxx*/ comment. I don't know
what the author want chooseVideoDriver function to do, but if it is
"chose a driver whatever info is missing or error occurred", the
platform device selection code should be called in the worst case
rather than just let it be like what's now in PS3.