Changed location of LibsecretLoader

components/os_crypt/libsecret_loader.cc

+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[vabr (Chromium), 2016/04/28 07:53:21]

nit: No (c).

[cfroussios, 2016/04/28 12:18:10]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/os_crypt/libsecret_loader.h"
+
+#include <dlfcn.h>
+
+#include "base/logging.h"
+
+typeof(&::secret_password_store_sync)

[vabr (Chromium), 2016/04/28 07:53:20]

Please use C++11 decltype instead of the non-standard typeof. decltype is
already allowed to use in Chromium (http://chromium-cpp.appspot.com/), it might
not have been at the time the moved code was written.

[cfroussios, 2016/04/28 12:18:11]

Done.

+    LibsecretLoader::secret_password_store_sync;
+typeof(&::secret_service_search_sync)
+    LibsecretLoader::secret_service_search_sync;
+typeof(&::secret_password_clear_sync)
+    LibsecretLoader::secret_password_clear_sync;
+typeof(&::secret_item_get_secret) LibsecretLoader::secret_item_get_secret;
+typeof(&::secret_value_get_text) LibsecretLoader::secret_value_get_text;
+typeof(&::secret_item_get_attributes)
+    LibsecretLoader::secret_item_get_attributes;
+typeof(&::secret_item_load_secret_sync)
+    LibsecretLoader::secret_item_load_secret_sync;
+typeof(&::secret_value_unref) LibsecretLoader::secret_value_unref;
+
+bool LibsecretLoader::libsecret_loaded = false;
+
+const LibsecretLoader::FunctionInfo LibsecretLoader::functions[] = {
+    {"secret_password_store_sync",
+     reinterpret_cast<void**>(&secret_password_store_sync)},
+    {"secret_service_search_sync",
+     reinterpret_cast<void**>(&secret_service_search_sync)},
+    {"secret_password_clear_sync",
+     reinterpret_cast<void**>(&secret_password_clear_sync)},
+    {"secret_item_get_secret",
+     reinterpret_cast<void**>(&secret_item_get_secret)},
+    {"secret_value_get_text", reinterpret_cast<void**>(&secret_value_get_text)},
+    {"secret_item_get_attributes",
+     reinterpret_cast<void**>(&secret_item_get_attributes)},
+    {"secret_item_load_secret_sync",
+     reinterpret_cast<void**>(&secret_item_load_secret_sync)},
+    {"secret_value_unref", reinterpret_cast<void**>(&secret_value_unref)},
+    {nullptr, nullptr}};
+
+bool LibsecretLoader::LoadLibsecret() {

[vabr (Chromium), 2016/04/28 07:53:21]

nit: For all static methods the convention is to add a line
// static
just before the first line of the definition.

[cfroussios, 2016/04/28 12:18:10]

Done.

+  if (libsecret_loaded)
+    return true;
+
+  void* handle = dlopen("libsecret-1.so.0", RTLD_NOW | RTLD_GLOBAL);
+  if (!handle) {
+    // We wanted to use libsecret, but we couldn't load it. Warn, because
+    // either the user asked for this, or we autodetected it incorrectly. (Or
+    // the system has broken libraries, which is also good to warn about.)
+    LOG(WARNING) << "Could not load libsecret-1.so.0: " << dlerror();

[vabr (Chromium), 2016/04/28 07:53:21]

LOG(WARNING) is discouraged [1]. Please replace this with VLOG(1) and add
// TODO(crbug.com/607435) Channel this message to the user-facing log.

[cfroussios, 2016/04/28 12:18:10]

Done.

+    return false;
+  }
+
+  for (size_t i = 0; functions[i].name; ++i) {

[vabr (Chromium), 2016/04/28 07:53:21]

If you use the range-based for loop, you can get rid of the nullptr,nullptr pair
in functions[]:

for (const auto& function : functions) { ... }

[cfroussios, 2016/04/28 12:18:10]

Done.

+    dlerror();
+    *functions[i].pointer = dlsym(handle, functions[i].name);
+    const char* error = dlerror();
+    if (error) {
+      VLOG(1) << "Unable to load symbol " << functions[i].name << ": " << error;
+      dlclose(handle);
+      return false;
+    }
+  }
+
+  libsecret_loaded = true;
+  // We leak the library handle. That's OK: this function is called only once.
+  return true;
+}
+