Tidy FileVersionInfoWin.
- Refactor GetValue() to use an existing struct.
- Make the parameter names in the .cc / .h files match for GetValue().
- Mark GetValue() and GetStringValue() const.
- Make all ::VerQueryValue() callers consistent.
- Add a comment to note |fixed_file_info_| is never nullptr.
- Fix various nits.
Change-Id: I4e73bf5791045005c2cfdeb6a039815c39e953ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837490
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702890}
diff --git a/base/file_version_info_win.cc b/base/file_version_info_win.cc
index 1572d0c..8b8e5c6 100644
--- a/base/file_version_info_win.cc
+++ b/base/file_version_info_win.cc
@@ -7,6 +7,8 @@
#include <windows.h>
#include <stddef.h>
+#include <utility>
+
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
@@ -15,8 +17,6 @@
#include "base/threading/scoped_blocking_call.h"
#include "base/win/resource_util.h"
-using base::FilePath;
-
namespace {
struct LanguageAndCodePage {
@@ -24,26 +24,23 @@
WORD code_page;
};
-// Returns the \\VarFileInfo\\Translation value extracted from the
+// Returns the \VarFileInfo\Translation value extracted from the
// VS_VERSION_INFO resource in |data|.
LanguageAndCodePage* GetTranslate(const void* data) {
- LanguageAndCodePage* translate = nullptr;
- UINT length;
- if (::VerQueryValue(data, L"\\VarFileInfo\\Translation",
- reinterpret_cast<void**>(&translate), &length)) {
- return translate;
- }
+ static constexpr wchar_t kTranslation[] = L"\\VarFileInfo\\Translation";
+ LPVOID translate = nullptr;
+ UINT dummy_size;
+ if (::VerQueryValue(data, kTranslation, &translate, &dummy_size))
+ return static_cast<LanguageAndCodePage*>(translate);
return nullptr;
}
-VS_FIXEDFILEINFO* GetVsFixedFileInfo(const void* data) {
- VS_FIXEDFILEINFO* fixed_file_info = nullptr;
- UINT length;
- if (::VerQueryValue(data, L"\\", reinterpret_cast<void**>(&fixed_file_info),
- &length)) {
- return fixed_file_info;
- }
- return nullptr;
+const VS_FIXEDFILEINFO& GetVsFixedFileInfo(const void* data) {
+ static constexpr wchar_t kRoot[] = L"\\";
+ LPVOID fixed_file_info = nullptr;
+ UINT dummy_size;
+ CHECK(::VerQueryValue(data, kRoot, &fixed_file_info, &dummy_size));
+ return *static_cast<VS_FIXEDFILEINFO*>(fixed_file_info);
}
} // namespace
@@ -70,13 +67,13 @@
// static
std::unique_ptr<FileVersionInfo> FileVersionInfo::CreateFileVersionInfo(
- const FilePath& file_path) {
+ const base::FilePath& file_path) {
return FileVersionInfoWin::CreateFileVersionInfoWin(file_path);
}
// static
std::unique_ptr<FileVersionInfoWin>
-FileVersionInfoWin::CreateFileVersionInfoWin(const FilePath& file_path) {
+FileVersionInfoWin::CreateFileVersionInfoWin(const base::FilePath& file_path) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
@@ -140,55 +137,46 @@
}
bool FileVersionInfoWin::GetValue(const base::char16* name,
- base::string16* value_str) {
- WORD lang_codepage[8];
- size_t i = 0;
- // Use the language and codepage from the DLL.
- lang_codepage[i++] = language_;
- lang_codepage[i++] = code_page_;
- // Use the default language and codepage from the DLL.
- lang_codepage[i++] = ::GetUserDefaultLangID();
- lang_codepage[i++] = code_page_;
- // Use the language from the DLL and Latin codepage (most common).
- lang_codepage[i++] = language_;
- lang_codepage[i++] = 1252;
- // Use the default language and Latin codepage (most common).
- lang_codepage[i++] = ::GetUserDefaultLangID();
- lang_codepage[i++] = 1252;
+ base::string16* value) const {
+ const struct LanguageAndCodePage lang_codepages[] = {
+ // Use the language and codepage from the DLL.
+ {language_, code_page_},
+ // Use the default language and codepage from the DLL.
+ {::GetUserDefaultLangID(), code_page_},
+ // Use the language from the DLL and Latin codepage (most common).
+ {language_, 1252},
+ // Use the default language and Latin codepage (most common).
+ {::GetUserDefaultLangID(), 1252},
+ };
- i = 0;
- while (i < base::size(lang_codepage)) {
+ for (const auto& lang_codepage : lang_codepages) {
wchar_t sub_block[MAX_PATH];
- WORD language = lang_codepage[i++];
- WORD code_page = lang_codepage[i++];
_snwprintf_s(sub_block, MAX_PATH, MAX_PATH,
- L"\\StringFileInfo\\%04x%04x\\%ls", language, code_page,
- base::as_wcstr(name));
- LPVOID value = NULL;
+ L"\\StringFileInfo\\%04x%04x\\%ls", lang_codepage.language,
+ lang_codepage.code_page, base::as_wcstr(name));
+ LPVOID value_ptr = nullptr;
uint32_t size;
- BOOL r = ::VerQueryValue(data_, sub_block, &value, &size);
- if (r && value) {
- value_str->assign(static_cast<base::char16*>(value));
+ BOOL r = ::VerQueryValue(data_, sub_block, &value_ptr, &size);
+ if (r && value_ptr && size) {
+ value->assign(static_cast<base::char16*>(value_ptr), size - 1);
return true;
}
}
return false;
}
-base::string16 FileVersionInfoWin::GetStringValue(const base::char16* name) {
+base::string16 FileVersionInfoWin::GetStringValue(
+ const base::char16* name) const {
base::string16 str;
- if (GetValue(name, &str))
- return str;
- else
- return base::string16();
+ GetValue(name, &str);
+ return str;
}
base::Version FileVersionInfoWin::GetFileVersion() const {
- return base::Version(
- std::vector<uint32_t>{HIWORD(fixed_file_info_->dwFileVersionMS),
- LOWORD(fixed_file_info_->dwFileVersionMS),
- HIWORD(fixed_file_info_->dwFileVersionLS),
- LOWORD(fixed_file_info_->dwFileVersionLS)});
+ return base::Version({HIWORD(fixed_file_info_.dwFileVersionMS),
+ LOWORD(fixed_file_info_.dwFileVersionMS),
+ HIWORD(fixed_file_info_.dwFileVersionLS),
+ LOWORD(fixed_file_info_.dwFileVersionLS)});
}
FileVersionInfoWin::FileVersionInfoWin(std::vector<uint8_t>&& data,
diff --git a/base/file_version_info_win.h b/base/file_version_info_win.h
index 30d4756c..f0dbde8 100644
--- a/base/file_version_info_win.h
+++ b/base/file_version_info_win.h
@@ -38,12 +38,13 @@
base::string16 file_description() override;
base::string16 file_version() override;
- // Lets you access other properties not covered above.
- bool GetValue(const base::char16* name, base::string16* value);
+ // Lets you access other properties not covered above. |value| is only
+ // modified if GetValue() returns true.
+ bool GetValue(const base::char16* name, base::string16* value) const;
// Similar to GetValue but returns a string16 (empty string if the property
// does not exist).
- base::string16 GetStringValue(const base::char16* name);
+ base::string16 GetStringValue(const base::char16* name) const;
// Get file version number in dotted version format.
base::Version GetFileVersion() const;
@@ -67,8 +68,8 @@
const WORD language_;
const WORD code_page_;
- // This is a pointer into |data_| if it exists. Otherwise nullptr.
- const VS_FIXEDFILEINFO* const fixed_file_info_;
+ // This is a reference for a portion of |data_|.
+ const VS_FIXEDFILEINFO& fixed_file_info_;
DISALLOW_COPY_AND_ASSIGN(FileVersionInfoWin);
};