Skip to content

Commit 50c6c2e

Browse files
committed
[Runtime][Win32] Tweak SymbolInfo slightly based on review comments.
Prefer `ZeroMemory()` to `::memset()` here. Use `sizeof(*wszBuffer)` instead of `sizeof(WCHAR)`, just in case. Don't use `*this=other`, because that motivates tests around `::free()`, but instead pull the shared code out into some private functions. Also, fix `SymbolInfo()` to initialize the pointer members. rdar://130992923
1 parent 7b7aa5a commit 50c6c2e

File tree

2 files changed

+25
-21
lines changed

2 files changed

+25
-21
lines changed

stdlib/public/runtime/SymbolInfo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ struct Win32ModuleInfo {
7777
};
7878

7979
// Get the filename and base of the module that contains the specified
80-
// address. N.B. This returns a malloc()ed copy of the filename in the
80+
// address. N.B. This returns a `malloc`-ed copy of the filename in the
8181
// Win32ModuleInfo struct; you are responsible for freeing that.
8282
static Win32ModuleInfo moduleInfoFromAddress(const void *address) {
8383
HMODULE hModule;
@@ -89,12 +89,12 @@ static Win32ModuleInfo moduleInfoFromAddress(const void *address) {
8989

9090
if (!GetModuleInformation(GetCurrentProcess(), hModule,
9191
&mi, sizeof(mi))) {
92-
::memset(&mi, 0, sizeof(mi));
92+
ZeroMemory(&mi, sizeof(mi));
9393
}
9494

9595
WCHAR wszBuffer[256];
9696
LPWSTR pwszFileName = wszBuffer;
97-
DWORD dwCapacity = sizeof(wszBuffer) / sizeof(WCHAR);
97+
DWORD dwCapacity = sizeof(wszBuffer) / sizeof(*wszBuffer);
9898
DWORD dwRet = GetModuleFileNameW(hModule, pwszFileName, dwCapacity);
9999

100100
if (dwRet == dwCapacity) {

stdlib/public/runtime/SymbolInfo.h

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,38 +61,40 @@ struct SymbolInfo {
6161
_moduleFileName(moduleFileName),
6262
_moduleBaseAddress(moduleBaseAddress)
6363
{}
64+
65+
void initializeFrom(const SymbolInfo &other) {
66+
_symbolAddress = other._symbolAddress;
67+
_symbolName = ::strdup(other._symbolName);
68+
_moduleFileName = ::strdup(other._moduleFileName);
69+
_moduleBaseAddress = other._moduleBaseAddress;
70+
}
71+
72+
void deinitialize() {
73+
::free((void *)_moduleFileName);
74+
::free((void *)_symbolName);
75+
_moduleFileName = nullptr;
76+
_symbolName = nullptr;
77+
}
6478
#endif
6579

6680
public:
67-
SymbolInfo() {}
68-
6981
#if defined(_WIN32) && !defined(__CYGWIN__)
82+
SymbolInfo() : _symbolName(nullptr), _moduleFileName(nullptr) {}
83+
7084
SymbolInfo(const SymbolInfo &other) {
71-
_symbolName = nullptr;
72-
_moduleFileName = nullptr;
73-
*this = other;
85+
initializeFrom(other);
7486
}
7587
SymbolInfo(SymbolInfo &&other) {
7688
*this = std::move(other);
7789
}
7890
~SymbolInfo() {
79-
if (_moduleFileName)
80-
::free((void *)_moduleFileName);
81-
if (_symbolName)
82-
::free((void *)_symbolName);
91+
deinitialize();
8392
}
8493

8594
SymbolInfo &operator=(const SymbolInfo &other) {
8695
if (this != &other) {
87-
if (_moduleFileName)
88-
::free((void *)_moduleFileName);
89-
if (_symbolName)
90-
::free((void *)_symbolName);
91-
92-
_symbolAddress = other._symbolAddress;
93-
_symbolName = ::strdup(other._symbolName);
94-
_moduleFileName = ::strdup(other._moduleFileName);
95-
_moduleBaseAddress = other._moduleBaseAddress;
96+
deinitialize();
97+
initializeFrom(other);
9698
}
9799

98100
return *this;
@@ -109,6 +111,8 @@ struct SymbolInfo {
109111

110112
return *this;
111113
}
114+
#else
115+
SymbolInfo() {}
112116
#endif
113117

114118
/// Get the file name of the image where the symbol was found.

0 commit comments

Comments
 (0)