Making sure that base::GetModuleFromAddress() does not increment the module reference counter sinse… (issue 10391169)

11 views
Skip to first unread message

alex...@chromium.org

unread,
May 16, 2012, 4:22:55 PM5/16/12
to j...@chromium.org, w...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org
Reviewers: jar, Wez,

Message:
Please take a look.

Description:
Making sure that base::GetModuleFromAddress() does not increment the module
reference counter sinse the rest of the code assume it does not.

The original change was reverted by
https://chromiumcodereview.appspot.com/10384209.

BUG=124091


Please review this at https://chromiumcodereview.appspot.com/10391169/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M base/process_util.h
M base/process_util_win.cc


Index: base/process_util.h
diff --git a/base/process_util.h b/base/process_util.h
index
074312bfde58b36ca425551b1e278c6999bd8881..15d6b68cda5f492a66ec97cb85c5d184938457a3
100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -146,7 +146,8 @@ BASE_EXPORT ProcessId GetCurrentProcId();
BASE_EXPORT ProcessHandle GetCurrentProcessHandle();

#if defined(OS_WIN)
-// Returns the module handle to which an address belongs.
+// Returns the module handle to which an address belongs. The reference
counter
+// of the module is not incremented.
BASE_EXPORT HMODULE GetModuleFromAddress(void* address);
#endif

Index: base/process_util_win.cc
diff --git a/base/process_util_win.cc b/base/process_util_win.cc
index
4fab4c68efc1da792e06b59f3ca07a5ed58ead11..9b2e100439cd24bdd060139a433eed2b3c91ceda
100644
--- a/base/process_util_win.cc
+++ b/base/process_util_win.cc
@@ -176,13 +176,14 @@ ProcessHandle GetCurrentProcessHandle() {
}

HMODULE GetModuleFromAddress(void* address) {
- HMODULE hinst = NULL;
- if (!::GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
+ HMODULE instance = NULL;
+ if (!::GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS |
+ GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
static_cast<char*>(address),
- &hinst)) {
+ &instance)) {
NOTREACHED();
}
- return hinst;
+ return instance;
}

bool OpenProcessHandle(ProcessId pid, ProcessHandle* handle) {


w...@chromium.org

unread,
May 16, 2012, 4:27:50 PM5/16/12
to alex...@chromium.org, j...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org
LGTM w/ one nit, and in the description:

typo: sinse -> sinse

Suggested description:
"Fix base::GetModuleFromAddress() not to increment the module's reference
count,
to match the behaviour expected by calling code."


https://chromiumcodereview.appspot.com/10391169/diff/1/base/process_util.h
File base/process_util.h (right):

https://chromiumcodereview.appspot.com/10391169/diff/1/base/process_util.h#newcode149
base/process_util.h:149: // Returns the module handle to which an
address belongs. The reference counter
nit: counter -> count

https://chromiumcodereview.appspot.com/10391169/

alex...@chromium.org

unread,
May 16, 2012, 4:32:13 PM5/16/12
to j...@chromium.org, w...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org
FYI


https://chromiumcodereview.appspot.com/10391169/diff/1/base/process_util.h
File base/process_util.h (right):

https://chromiumcodereview.appspot.com/10391169/diff/1/base/process_util.h#newcode149
base/process_util.h:149: // Returns the module handle to which an
address belongs. The reference counter
On 2012/05/16 20:27:50, Wez wrote:
> nit: counter -> count

Done.

https://chromiumcodereview.appspot.com/10391169/

j...@chromium.org

unread,
May 16, 2012, 6:16:00 PM5/16/12
to alex...@chromium.org, w...@chromium.org, rva...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org
I'd like Ricardo to have a look at this, as he's much much more familiar
with
the Window-isms.

https://chromiumcodereview.appspot.com/10391169/

rva...@chromium.org

unread,
May 16, 2012, 6:21:50 PM5/16/12
to alex...@chromium.org, j...@chromium.org, w...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org

j...@chromium.org

unread,
May 16, 2012, 6:41:05 PM5/16/12
to alex...@chromium.org, w...@chromium.org, rva...@chromium.org, chromium...@chromium.org, erikw...@chromium.org, bret...@chromium.org
Reply all
Reply to author
Forward
0 new messages