Bug 23634

Summary: video-xgi: xgi_drv does not have a xgiModuleData data object.
Product: xorg Reporter: Haruo Tomita <haruo.tomita>
Component: Driver/XGIAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium CC: haruo.tomita, jesserayadkins
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
See Also: https://bugs.freedesktop.org/show_bug.cgi?id=41208
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
Xorg.0.log
none
patch none

Description Haruo Tomita 2009-09-01 17:33:24 UTC
Created attachment 29085 [details]
Xorg.0.log

When the xgi driver is loaded with Ferora12 Alpha, 
it becomes the following errors. 

(II) Loading extension DRI2
(II) LoadModule: "xgi"
(II) Loading /usr/lib/xorg/modules/drivers/xgi_drv.so
(EE) LoadModule: Module xgi does not have a xgiModuleData data object.
(II) UnloadModule: "xgi"
(II) Unloading /usr/lib/xorg/modules/drivers/xgi_drv.so
(EE) Failed to load module "xgi" (invalid module, 0)

See attached file: Xorg.0.log

It seems that it is a cause not to export xgiModuleData. 
Moreover, I think that I should remove an unnecessary loader symbol lists.
Comment 1 Haruo Tomita 2009-09-01 17:41:22 UTC
This is an experimental patch. The patch is as follows. 
The xgi_drv is working fine with this patch. 

--- xf86-video-xgi-20090825/src/xgi_driver.c.orig	2009-09-01 09:25:18.922972383 +0900
+++ xf86-video-xgi-20090825/src/xgi_driver.c	2009-09-01 12:13:02.563995782 +0900
@@ -180,7 +180,7 @@ static const struct pci_id_match xgi_dev
  * an upper-case version of the driver name.
  */
 
-DriverRec XGI = {
+_X_EXPORT DriverRec XGI = {
     XGI_CURRENT_VERSION,
     XGI_DRIVER_NAME,
     XGIIdentify,
@@ -216,90 +216,6 @@ static PciChipsets XGIPciChipsets[] = {
     {-1, -1, RES_UNDEFINED}
 };
 
-static const char *xaaSymbols[] = {
-    "XAACopyROP",
-    "XAACreateInfoRec",
-    "XAADestroyInfoRec",
-    "XAAFillMono8x8PatternRects",
-    "XAAPatternROP",
-    "XAAHelpPatternROP",
-    "XAAInit",
-    NULL
-};
-
-static const char *vgahwSymbols[] = {
-    "vgaHWFreeHWRec",
-    "vgaHWGetHWRec",
-    "vgaHWGetIOBase",
-    "vgaHWGetIndex",
-    "vgaHWInit",
-    "vgaHWLock",
-    "vgaHWMapMem",
-    "vgaHWUnmapMem",
-    "vgaHWProtect",
-    "vgaHWRestore",
-    "vgaHWSave",
-    "vgaHWSaveScreen",
-    "vgaHWUnlock",
-    NULL
-};
-
-static const char *fbSymbols[] = {
-    "fbPictureInit",
-    "fbScreenInit",
-    NULL
-};
-
-static const char *shadowSymbols[] = {
-    "ShadowFBInit",
-    NULL
-};
-
-static const char *ramdacSymbols[] = {
-    "xf86CreateCursorInfoRec",
-    "xf86DestroyCursorInfoRec",
-    "xf86InitCursor",
-    NULL
-};
-
-
-static const char *ddcSymbols[] = {
-    "xf86PrintEDID",
-    "xf86SetDDCproperties",
-    "xf86InterpretEDID",
-    NULL
-};
-
-
-/* static const char *i2cSymbols[] = {
-    "xf86I2CBusInit",
-    "xf86CreateI2CBusRec",
-    NULL
-}; */
-
-static const char *int10Symbols[] = {
-    "xf86FreeInt10",
-    "xf86InitInt10",
-    "xf86ExecX86int10",
-    NULL
-};
-
-static const char *vbeSymbols[] = {
-    "VBEExtendedInit",
-    "vbeDoEDID",
-    "vbeFree",
-    "VBEGetVBEInfo",
-    "VBEFreeVBEInfo",
-    "VBEGetModeInfo",
-    "VBEFreeModeInfo",
-    "VBESaveRestore",
-    "VBESetVBEMode",
-    "VBEGetVBEMode",
-    "VBESetDisplayStart",
-    "VBESetGetLogicalScanlineLength",
-    NULL
-};
-
 #ifdef XF86DRI
 static const char *drmSymbols[] = {
     "drmAddMap",
@@ -358,7 +274,7 @@ static XF86ModuleVersionInfo xgiVersRec 
     {0, 0, 0, 0}
 };
 
-XF86ModuleData xgiModuleData = { &xgiVersRec, xgiSetup, NULL };
+_X_EXPORT XF86ModuleData xgiModuleData = { &xgiVersRec, xgiSetup, NULL };
 
 /*** static string ***/
 #ifdef XGIMERGED
@@ -475,13 +391,6 @@ xgiSetup(pointer module, pointer opts, i
     if (!setupDone) {
         setupDone = TRUE;
         xf86AddDriver(&XGI, module, HaveDriverFuncs);
-        LoaderRefSymLists(vgahwSymbols, fbSymbols, xaaSymbols,
-                          shadowSymbols, ramdacSymbols, ddcSymbols,
-                          vbeSymbols, int10Symbols,
-#ifdef XF86DRI
-                          drmSymbols, driSymbols,
-#endif
-                          NULL);
         return (pointer) TRUE;
     }
 
@@ -1455,7 +1364,6 @@ XGIInternalDDC(ScrnInfoPtr pScrn, int cr
     }
 
     if (xf86LoadSubModule(pScrn, "int10")) {
-        xf86LoaderReqSymLists(int10Symbols, NULL);
         pInt = xf86InitInt10(pXGI->pEnt->index);
         if (pInt == NULL) {
             xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
@@ -1507,8 +1415,6 @@ XGIInternalDDC(ScrnInfoPtr pScrn, int cr
             }
 #endif /* DEBUG3 */
 
-            xf86LoaderReqSymLists(ddcSymbols, NULL);
-
 	    /* Jong 09/04/2007; Alan fixed abnormal EDID data */
 	    /* pMonitor = xf86InterpretEDID(pScrn->scrnIndex, buffer) ; */
             if ( (buffer[0]==0) && (buffer[7]==0) )
@@ -2018,8 +1924,6 @@ XGIDDCPreInit(ScrnInfoPtr pScrn)
 
         if (xf86LoadSubModule(pScrn, "ddc")) {
 
-            xf86LoaderReqSymLists(ddcSymbols, NULL);
-
             pMonitor = XGIInternalDDC(pScrn, 0);
             if (pMonitor == NULL) {
                 xf86DrvMsg(pScrn->scrnIndex, X_INFO,
@@ -2349,8 +2253,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
         return FALSE;
     }
 
-    xf86LoaderReqSymLists(vgahwSymbols, NULL);
-
     /* Due to the liberal license terms this is needed for
      * keeping the copyright notice readable and intact in
      * binary distributions. Removing this is a copyright
@@ -2445,7 +2347,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
                    "Initializing display adapter through int10\n");
 
         if (xf86LoadSubModule(pScrn, "int10")) {
-            xf86LoaderReqSymLists(int10Symbols, NULL);
             pXGI->pInt = xf86InitInt10(pXGI->pEnt->index);
         }
         else {
@@ -2477,8 +2378,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
         return FALSE;
     }
 
-    xf86LoaderReqSymLists(ramdacSymbols, NULL);
-
     /* Set pScrn->monitor */
     pScrn->monitor = pScrn->confScreen->monitor;
 
@@ -3333,7 +3232,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
 #if !defined(__powerpc__)
     /* Now load and initialize VBE module. */
     if (xf86LoadSubModule(pScrn, "vbe")) {
-        xf86LoaderReqSymLists(vbeSymbols, NULL);
         pXGI->pVbe = VBEExtendedInit(pXGI->pInt, pXGI->pEnt->index,
                                      SET_BIOS_SCRATCH | RESTORE_BIOS_SCRATCH);
         if (!pXGI->pVbe) {
@@ -3711,7 +3609,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
         XGIFreeRec(pScrn);
         return FALSE;
     }
-    xf86LoaderReqSymLists(fbSymbols, NULL);
 
     /* Load XAA if needed */
     if (!pXGI->NoAccel) {
@@ -3728,7 +3625,6 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
             XGIFreeRec(pScrn);
             return FALSE;
         }
-        xf86LoaderReqSymLists(xaaSymbols, NULL);
     }
 
     /* Load shadowfb if needed */
@@ -3745,20 +3641,14 @@ XGIPreInit(ScrnInfoPtr pScrn, int flags)
             XGIFreeRec(pScrn);
             return FALSE;
         }
-        xf86LoaderReqSymLists(shadowSymbols, NULL);
     }
 
     /* Load the dri module if requested. */
 #ifdef XF86DRI
     if(pXGI->loadDRI) {
-        if (xf86LoadSubModule(pScrn, "dri")) {
-            xf86LoaderReqSymLists(driSymbols, drmSymbols, NULL);
-        }
-        else {
-            if (!IS_DUAL_HEAD(pXGI))
-                xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-                           "Remove >Load \"dri\"< from the Module section of your XF86Config file\n");
-        }
+        if (!IS_DUAL_HEAD(pXGI))
+            xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+                       "Remove >Load \"dri\"< from the Module section of your XF86Config file\n");
     }
 #endif
 
@@ -4253,7 +4143,6 @@ XGIScreenInit(int scrnIndex, ScreenPtr p
 #if !defined(__powerpc__)
     if (!IS_DUAL_HEAD(pXGI) || !IS_SECOND_HEAD(pXGI)) {
         if (xf86LoadSubModule(pScrn, "vbe")) {
-            xf86LoaderReqSymLists(vbeSymbols, NULL);
             pXGI->pVbe = VBEExtendedInit(NULL, pXGI->pEnt->index,
                                          SET_BIOS_SCRATCH |
                                          RESTORE_BIOS_SCRATCH);
Comment 2 Haruo Tomita 2009-09-01 17:43:04 UTC
Created attachment 29087 [details] [review]
patch
Comment 3 Jesse Adkins 2010-10-02 23:45:16 UTC
I've also noticed this module failing to build properly. However, there are a few issues :
* Patches to be accepted usually get mailed to xorg-devel@lists.freedesktop.org and copied to a bug report.
* Your patch isn't made by git format-patch. Patches made by git format-patch are much easier to commit. If you're not sure how to do it, you can take a look at http://www.x.org/wiki/Development/Documentation/SubmittingPatches or e-mail me about it.
* Your patch handles two issues. Generally, it's best if a patch handles a single thing, because it's easier to determine what's happened by going through the git log. The two issues that you're handling don't have to be fixed together. It's up to you what order you want to fix the issues in.

If you're still interested in having this work done, you can send two patches (one for fixing the module name, and one for removing the loader sym list stuff) to xorg-devel@lists.freedesktop.org. You may have to redo the patches against the latest git master.

I've added myself to the CC list since I'm also interested in having this be fixed.
Comment 4 Knut Petersen 2011-09-29 06:51:36 UTC
There is a longstanding bug in the module loader: If loading of
an existing modules fails, this is not detected but the loader tries
to lookup the ***ModuleData data object by using the handle NULL.
that obviously fails, and an erroneous message about a missing data
object is generated. Don´t believe in that "LoadModule: Module xgi does not have a xgiModuleData data object" message.

Have a look at the patch attached to Bug 41208.

cu,
 Knut
Comment 5 Daniel Stone 2011-09-29 07:36:17 UTC
(In reply to comment #4)
> There is a longstanding bug in the module loader: If loading of
> an existing modules fails, this is not detected but the loader tries
> to lookup the ***ModuleData data object by using the handle NULL.
> that obviously fails, and an erroneous message about a missing data
> object is generated. Don´t believe in that "LoadModule: Module xgi does not
> have a xgiModuleData data object" message.

Actually, if the problem is solved by adding an _X_EXPORT, then indeed the problem is exactly that: that the server isn't able to find a symbol called xgiModuleData in the module.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.