Linux: create a library loader code generator for dlopen and use it for libpci.

content/gpu/gpu_info_collector_linux.cc

Index: content/gpu/gpu_info_collector_linux.cc
diff --git a/content/gpu/gpu_info_collector_linux.cc b/content/gpu/gpu_info_collector_linux.cc
index c6f2c2bcd7a97e32db76975f388cf934b819ebd5..706004322c67291585de8e50b602d6b6033c6a22 100644
--- a/content/gpu/gpu_info_collector_linux.cc
+++ b/content/gpu/gpu_info_collector_linux.cc
@@ -4,15 +4,9 @@
 
 #include "content/gpu/gpu_info_collector.h"
 
-#include <dlfcn.h>
 #include <X11/Xlib.h>
 #include <vector>
 
-// TODO(phajdan.jr): Report problem upstream and make pci.h handle this.
-extern "C" {
-#include <pci/pci.h>
-}
-
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/file_util.h"
@@ -23,6 +17,7 @@ extern "C" {
 #include "base/string_split.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
+#include "libpci_loader.h"
 #include "third_party/libXNVCtrl/NVCtrl.h"
 #include "third_party/libXNVCtrl/NVCtrlLib.h"
 #include "ui/gl/gl_bindings.h"
@@ -33,28 +28,6 @@ extern "C" {
 
 namespace {
 
-// Define function types.
-typedef pci_access* (*FT_pci_alloc)();
-typedef void (*FT_pci_init)(pci_access*);
-typedef void (*FT_pci_cleanup)(pci_access*);
-typedef void (*FT_pci_scan_bus)(pci_access*);
-typedef void (*FT_pci_scan_bus)(pci_access*);
-typedef int (*FT_pci_fill_info)(pci_dev*, int);
-typedef char* (*FT_pci_lookup_name)(pci_access*, char*, int, int, ...);
-
-// This includes dynamically linked library handle and functions pointers from
-// libpci.
-struct PciInterface {
-  void* lib_handle;
-
-  FT_pci_alloc pci_alloc;
-  FT_pci_init pci_init;
-  FT_pci_cleanup pci_cleanup;
-  FT_pci_scan_bus pci_scan_bus;
-  FT_pci_fill_info pci_fill_info;
-  FT_pci_lookup_name pci_lookup_name;
-};
-
 // This checks if a system supports PCI bus.
 // We check the existence of /sys/bus/pci or /sys/bug/pci_express.
 bool IsPciSupported() {
@@ -64,68 +37,6 @@ bool IsPciSupported() {
           file_util::PathExists(pcie_path));
 }
 
-// This dynamically opens libpci and get function pointers we need.  Return
-// NULL if library fails to open or any functions can not be located.
-// Returned interface (if not NULL) should be deleted in FinalizeLibPci.
-PciInterface* InitializeLibPci(const char* lib_name) {
-  scoped_ptr<PciInterface> interface(new PciInterface);
-#if defined(DLOPEN_LIBPCI)
-  void* handle = dlopen(lib_name, RTLD_LAZY);
-  if (handle == NULL) {
-    VLOG(1) << "Failed to dlopen " << lib_name;
-    return NULL;
-  }
-  interface->lib_handle = handle;
-  interface->pci_alloc = reinterpret_cast<FT_pci_alloc>(
-      dlsym(handle, "pci_alloc"));
-  interface->pci_init = reinterpret_cast<FT_pci_init>(
-      dlsym(handle, "pci_init"));
-  interface->pci_cleanup = reinterpret_cast<FT_pci_cleanup>(
-      dlsym(handle, "pci_cleanup"));
-  interface->pci_scan_bus = reinterpret_cast<FT_pci_scan_bus>(
-      dlsym(handle, "pci_scan_bus"));
-  interface->pci_fill_info = reinterpret_cast<FT_pci_fill_info>(
-      dlsym(handle, "pci_fill_info"));
-  interface->pci_lookup_name = reinterpret_cast<FT_pci_lookup_name>(
-      dlsym(handle, "pci_lookup_name"));
-  if (interface->pci_alloc == NULL ||
-      interface->pci_init == NULL ||
-      interface->pci_cleanup == NULL ||
-      interface->pci_scan_bus == NULL ||
-      interface->pci_fill_info == NULL ||
-      interface->pci_lookup_name == NULL) {
-    VLOG(1) << "Missing required function(s) from " << lib_name;
-    dlclose(handle);
-    return NULL;
-  }
-#else  // !defined(DLOPEN_LIBPCI)
-  interface->lib_handle = NULL;
-  interface->pci_alloc = reinterpret_cast<FT_pci_alloc>(
-      &pci_alloc);
-  interface->pci_init = reinterpret_cast<FT_pci_init>(
-      &pci_init);
-  interface->pci_cleanup = reinterpret_cast<FT_pci_cleanup>(
-      &pci_cleanup);
-  interface->pci_scan_bus = reinterpret_cast<FT_pci_scan_bus>(
-      &pci_scan_bus);
-  interface->pci_fill_info = reinterpret_cast<FT_pci_fill_info>(
-      &pci_fill_info);
-  interface->pci_lookup_name = reinterpret_cast<FT_pci_lookup_name>(
-      &pci_lookup_name);
-#endif  // !defined(DLOPEN_LIBPCI)
-  return interface.release();
-}
-
-// This close the dynamically opened libpci and delete the interface.
-void FinalizeLibPci(PciInterface** interface) {
-#if defined(DLOPEN_LIBPCI)
-  DCHECK(interface && *interface && (*interface)->lib_handle);
-  dlclose((*interface)->lib_handle);
-#endif  // defined(DLOPEN_LIBPCI)
-  delete (*interface);
-  *interface = NULL;
-}
-
 // Scan /etc/ati/amdpcsdb.default for "ReleaseVersion".
 // Return empty string on failing.
 std::string CollectDriverVersionATI() {
@@ -263,22 +174,22 @@ bool CollectVideoCardInfo(content::GPUInfo* gpu_info) {
   }
 
   // TODO(zmo): be more flexible about library name.
-  PciInterface* interface = InitializeLibPci("libpci.so.3");
-  if (interface == NULL)
-    interface = InitializeLibPci("libpci.so");
-  if (interface == NULL) {
-    VLOG(1) << "Failed to locate libpci";
+  LibPciLoader libpci_loader;
+  if (!libpci_loader.Load("libpci.so.3") &&
+      !libpci_loader.Load("libpci.so")) {

[Mark Mentovai, 2012/11/27 21:01:18]

In a world where libpci.so.3 isn’t present because everyone’s moved on to
libpci.so.5 or whatever comes next, this will load a library that has
intentionally had its version number bumped because of a binary incompatibility.
Why are you falling back to libpci.so? Do you have to? Can you stop? It’s at
least worth a comment.

[Zhenyao Mo, 2012/11/27 21:18:50]

We search for libpci.so.3 first because it's the mostly available one and we
have tested our code with it.

By falling back to libpci.so, at least before chromium upgrade to use a newer
lib here, users can create a symlink.

[Paweł Hajdan Jr., 2012/11/27 22:27:24]

I'll let you sort that out with Mark. I think that loading a
non-binary-compatible library (and that's what a change of the name means) is
asking for trouble. With dlsym we have no guarantees it has the interface we
expect. I'd prefer to only leave libpci.so.3 in.

Note that this was here before, I'm not changing the logic here. I'm fine with
doing a small cleanup though if needed.

[Mark Mentovai, 2012/11/27 23:09:11]

Further discussion at crbug.com/162991. No need to hold this change up for that.

+    VLOG(1) << "Failed to locate libpci.";

[Mark Mentovai, 2012/11/27 21:01:18]

We should be shortening log strings, not making them worse.

[Paweł Hajdan Jr., 2012/11/27 22:27:24]

Done.

     return false;
   }
 
-  pci_access* access = (interface->pci_alloc)();
+  pci_access* access = (libpci_loader.pci_alloc)();
   DCHECK(access != NULL);
-  (interface->pci_init)(access);
-  (interface->pci_scan_bus)(access);
+  (libpci_loader.pci_init)(access);
+  (libpci_loader.pci_scan_bus)(access);
   bool primary_gpu_identified = false;
   for (pci_dev* device = access->devices;
        device != NULL; device = device->next) {
-    (interface->pci_fill_info)(device, 33);  // Fill the IDs and class fields.
+    // Fill the IDs and class fields.
+    (libpci_loader.pci_fill_info)(device, 33);
     // TODO(zmo): there might be other classes that qualify as display devices.
     if (device->device_class != 0x0300)  // Device class is DISPLAY_VGA.
       continue;
@@ -292,19 +203,19 @@ bool CollectVideoCardInfo(content::GPUInfo* gpu_info) {
     // The current implementation of pci_lookup_name returns the same pointer
     // as the passed in upon success, and a different one (NULL or a pointer
     // to an error message) upon failure.
-    if ((interface->pci_lookup_name)(access,
-                                     buffer.get(),
-                                     buffer_size,
-                                     1,
-                                     device->vendor_id) == buffer.get()) {
+    if ((libpci_loader.pci_lookup_name)(access,
+                                        buffer.get(),
+                                        buffer_size,
+                                        1,
+                                        device->vendor_id) == buffer.get()) {
       gpu.vendor_string = buffer.get();
     }
-    if ((interface->pci_lookup_name)(access,
-                                     buffer.get(),
-                                     buffer_size,
-                                     2,
-                                     device->vendor_id,
-                                     device->device_id) == buffer.get()) {
+    if ((libpci_loader.pci_lookup_name)(access,
+                                        buffer.get(),
+                                        buffer_size,
+                                        2,
+                                        device->vendor_id,
+                                        device->device_id) == buffer.get()) {
       std::string device_string = buffer.get();
       size_t begin = device_string.find_first_of('[');
       size_t end = device_string.find_last_of(']');
@@ -341,8 +252,7 @@ bool CollectVideoCardInfo(content::GPUInfo* gpu_info) {
       gpu_info->amd_switchable = true;
   }
 
-  (interface->pci_cleanup)(access);
-  FinalizeLibPci(&interface);
+  (libpci_loader.pci_cleanup)(access);
   return (primary_gpu_identified);
 }