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

tools/generate_library_loader/generate_library_loader.py

+#!/usr/bin/env python
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""
+Creates a library loader (a header and implementation file),
+which is a wrapper for dlopen or direct linking with given library.
+
+The loader makes it possible to have the same client code for both cases,
+and also makes it easier to write code using dlopen (and also provides
+a standard way to do so, and limits the ugliness just to generated files).
+
+For more info refer to http://crbug.com/162733 .
+"""
+
+
+import optparse
+import os.path
+import re
+import sys
+
+
+HEADER_TEMPLATE = """// This is generated file. Do not modify directly.
+// Path to the code generator: %(generator_path)s .
+
+#ifndef %(unique_prefix)s
+#define %(unique_prefix)s
+
+%(wrapped_header_include)s
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#ifdef %(unique_prefix)s_DLOPEN
+#include "base/native_library.h"
+#endif
+
+class %(class_name)s {
+ public:
+  %(class_name)s();
+  ~%(class_name)s();
+
+  bool Load(const std::string& library_name) WARN_UNUSED_RESULT;
+
+%(member_decls)s
+
+ private:
+  void CleanUp(bool unload);

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

Nothing ever calls CleanUp(false), so get rid of the argument.

[Paweł Hajdan Jr., 2012/11/28 01:05:52]

In theory when loaded_ is false that would happen.

+
+#ifdef %(unique_prefix)s_DLOPEN
+  base::NativeLibrary library_;
+#endif
+
+  bool loaded_;

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

In the DLOPEN variant, you can get rid of loaded_ and just use library_, right?

For the DT_NEEDED variant, you should hang on to loaded_ (see my comment
elsewhere about that—I think it’s a good idea to have the DT_NEEDED and DLOPEN
variants behave the same.)

[Paweł Hajdan Jr., 2012/11/28 01:05:52]

Yeah, I think it'd just further complicate the code and add more #ifdefs for a
negligible (IMHO) gain.

I can do that if you want, but I strongly prefer the current version because of
simplicity.

+
+  DISALLOW_COPY_AND_ASSIGN(%(class_name)s);
+};
+
+#endif  // %(unique_prefix)s
+"""
+
+
+HEADER_MEMBER_TEMPLATE = """  typeof(&::%(function_name)s) %(function_name)s;
+"""
+
+
+IMPL_TEMPLATE = """// This is generated file. Do not modify directly.
+// Path to the code generator: %(generator_path)s .
+
+#include "%(generated_header_name)s"
+
+#include "base/file_path.h"
+#include "base/logging.h"
+
+%(class_name)s::%(class_name)s() : loaded_(false) {
+}
+
+%(class_name)s::~%(class_name)s() {
+  CleanUp(loaded_);

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

I’d just leave the destructor at calling base::UnloadNativeLibrary. No need to
zero out the entire object on destruction. Chrome code doesn’t usually zero
things out in the destructor.

But is it OK to UnloadNativeLibrary if there is no library_?

[Paweł Hajdan Jr., 2012/11/28 01:05:52]

That's guarded by loaded_. I think you've asked for clean-up, so I implemented
it. :) It shouldn't hurt, and may help catching use-after-frees or something.

[Mark Mentovai, 2012/11/28 04:29:12]

Paweł Hajdan Jr. wrote:
I asked for clean-up during Load, not destroy…

+}
+
+bool %(class_name)s::Load(const std::string& library_name) {
+  if (loaded_) {
+    NOTREACHED();
+    return false;
+  }
+
+#ifdef %(unique_prefix)s_DLOPEN
+  library_ = base::LoadNativeLibrary(FilePath(library_name), NULL);
+  if (!library_)
+    return false;
+#endif
+
+%(member_init)s
+
+  loaded_ = true;
+  return true;
+}
+
+void %(class_name)s::CleanUp(bool unload) {
+#if defined(%(unique_prefix)s_DLOPEN)
+  if (unload) {
+    base::UnloadNativeLibrary(library_);
+    library_ = NULL;
+  }
+#endif
+  loaded_ = false;
+%(member_cleanup)s
+}
+"""
+
+IMPL_MEMBER_INIT_TEMPLATE = """
+#if defined(%(unique_prefix)s_DLOPEN)
+  this->%(function_name)s =

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

What’s all this->, then? We don’t normally write this-> for member variables in
non-static functions.

[Paweł Hajdan Jr., 2012/11/28 01:05:52]

Done.

+      reinterpret_cast<typeof(this->%(function_name)s)>(
+          base::GetFunctionPointerFromNativeLibrary(
+              library_, "%(function_name)s"));
+#elif defined(%(unique_prefix)s_DT_NEEDED)

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

Good name choice. It’s nice to have a name for this now. I can stop calling it
the “linked against” variant or something else awkward.

+  this->%(function_name)s = &::%(function_name)s;
+#else
+#error neither %(unique_prefix)s_DLOPEN nor %(unique_prefix)s_DT_NEEDED defined

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

I don’t think you need one #error for each function you try to load. That’s
needlessly going to bloat the build output, while not providing any error at all
in the (erroneous!) case where the script isn’t asked to produce any stubs.
Better idea: put the #error inside an #else if !defined() that can follow the
base::LoadNativeLibrary call.

[Paweł Hajdan Jr., 2012/11/28 01:05:52]

Done.

+#endif
+  if (!this->%(function_name)s) {
+    CleanUp(true);
+    return false;
+  }
+"""
+
+IMPL_MEMBER_CLEANUP_TEMPLATE = """  this->%(function_name)s = NULL;
+"""
+
+def main():
+  parser = optparse.OptionParser()
+  parser.add_option('--name')
+  parser.add_option('--output-cc')
+  parser.add_option('--output-h')
+  parser.add_option('--header')
+
+  parser.add_option('--use-extern-c', action='store_true', default=False)
+  parser.add_option('--link-directly', type=int, default=0)
+
+  options, args = parser.parse_args()
+
+  if not options.name:
+    parser.error('Missing --name parameter')
+  if not options.output_cc:
+    parser.error('Missing --output-cc parameter')
+  if not options.output_h:
+    parser.error('Missing --output-h parameter')
+  if not options.header:
+    parser.error('Missing --header paramater')
+  if not args:
+    parser.error('No function names specified')
+
+  # Make sure we are always dealing with an absolute path
+  # to avoid issues caused by different relative path roots.
+  options.output_cc = os.path.abspath(options.output_cc)
+  options.output_h = os.path.abspath(options.output_h)
+
+  # Create a unique prefix, e.g. for header guards.
+  # Stick a known string at the beginning to ensure this doesn't begin
+  # with an underscore, which is reserved for the C++ implementation.
+  unique_prefix = ('LIBRARY_LOADER_' +
+                   re.sub(r'[\W]', '_', options.output_h).upper())
+
+  member_decls = []
+  member_init = []
+  member_cleanup = []
+  for fn in args:
+    member_decls += HEADER_MEMBER_TEMPLATE % {
+      'function_name': fn,
+      'unique_prefix': unique_prefix
+    }
+    member_init += IMPL_MEMBER_INIT_TEMPLATE % {
+      'function_name': fn,
+      'unique_prefix': unique_prefix
+    }
+    member_cleanup += IMPL_MEMBER_CLEANUP_TEMPLATE % {
+      'function_name': fn,
+      'unique_prefix': unique_prefix
+    }
+
+  wrapped_header_include = '#include %s' % options.header
+
+  # Some libraries (e.g. libpci) have headers that cannot be included
+  # without extern "C", otherwise they cause the link to fail.
+  # TODO(phajdan.jr): This is a workaround for broken headers. Remove it.
+  if options.use_extern_c:
+    wrapped_header_include = 'extern "C" {\n%s\n}\n' % wrapped_header_include
+
+  # It seems cleaner just to have a single #define here and #ifdefs in bunch
+  # of places, rather than having a different set of templates, duplicating
+  # or complicating more code.
+  if options.link_directly == 0:
+    wrapped_header_include += '#define %s_DLOPEN\n' % unique_prefix
+  elif options.link_directly == 1:
+    wrapped_header_include += '#define %s_DT_NEEDED\n' % unique_prefix
+  else:
+    parser.error('Invalid value for --link-directly. Should be 0 or 1.')
+
+  # Make it easier for people to find the code generator just in case.
+  # Doing it this way is more maintainable, because it's going to work
+  # even if file gets moved without updating the contents.
+  generator_path = os.path.abspath(__file__)
+
+  header_contents = HEADER_TEMPLATE % {
+    'generator_path': generator_path,
+    'unique_prefix': unique_prefix,
+    'wrapped_header_include': wrapped_header_include,
+    'class_name': options.name,
+    'member_decls': ''.join(member_decls),
+  }
+
+  impl_contents = IMPL_TEMPLATE % {
+    'generator_path': generator_path,
+    'unique_prefix': unique_prefix,
+    'generated_header_name': options.output_h,
+    'class_name': options.name,
+    'member_init': ''.join(member_init),
+    'member_cleanup': ''.join(member_cleanup),
+  }
+
+  header_file = open(options.output_h, 'w')
+  try:
+    header_file.write(header_contents)
+  finally:
+    header_file.close()
+
+  impl_file = open(options.output_cc, 'w')
+  try:
+    impl_file.write(impl_contents)
+  finally:
+    impl_file.close()
+
+  return 0
+
+if __name__ == '__main__':
+  sys.exit(main())