win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez

Committed: https://crrev.com/d62f5447b35d5d8d1b3521a05b7ed312ab31dbd1
Cr-Commit-Position: refs/heads/master@{#384841}

[Nico, 2016-04-03 22:36:57]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

BUG=550065
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. Both Windows
(GetProcAddress()) and POSIX (dlsym()) require that conversions between
void* and function pointers work in practice, but on posix an explicit
reinterpret_cast is required, and it seems nice to have compilers behave
identically between platforms here (i.e. we should turn on clang/win's
-Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.
BUG=550065
==========

[Nico, 2016-04-03 22:37:43]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. Both Windows
(GetProcAddress()) and POSIX (dlsym()) require that conversions between
void* and function pointers work in practice, but on posix an explicit
reinterpret_cast is required, and it seems nice to have compilers behave
identically between platforms here (i.e. we should turn on clang/win's
-Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.
BUG=550065
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. Both Windows
(GetProcAddress()) and POSIX (dlsym()) require that conversions between
void* and function pointers work in practice, but on posix an explicit
reinterpret_cast is required, and it seems nice to have compilers behave
identically between platforms here (i.e. we should turn on clang/win's
-Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
==========

[Nico, 2016-04-04 00:34:37]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. Both Windows
(GetProcAddress()) and POSIX (dlsym()) require that conversions between
void* and function pointers work in practice, but on posix an explicit
reinterpret_cast is required, and it seems nice to have compilers behave
identically between platforms here (i.e. we should turn on clang/win's
-Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
==========

[Nico, 2016-04-04 00:35:43]

thakis@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2016-04-04 01:26:44]

I noticed this is happening in screen_capturer_win_magnifier.cc too, via a
single reinterpret_cast

https://codereview.chromium.org/1852143002/diff/140001/base/win/wrapped_windo...
File base/win/wrapped_window_proc.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/base/win/wrapped_windo...
base/win/wrapped_window_proc.cc:21: static_cast<char*>(address),
why double cast, why not just reinterpret_cast<> straight from window_proc here?

Is this just to make the static_cast explicit above, for the comment?

[Nico, 2016-04-04 01:38:47]

https://codereview.chromium.org/1852143002/diff/140001/base/win/wrapped_windo...
File base/win/wrapped_window_proc.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/base/win/wrapped_windo...
base/win/wrapped_window_proc.cc:21: static_cast<char*>(address),
On 2016/04/04 01:26:44, Will Harris wrote:
> why double cast, why not just reinterpret_cast<> straight from window_proc
here?
> 
> Is this just to make the static_cast explicit above, for the comment?

Yes. I had this inline here, but it looked cramped with the comment in here.

[Will Harris, 2016-04-04 02:02:23]

lgtm

[Nico, 2016-04-04 02:04:26]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2016-04-04 02:04:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1852143002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1852143002/140001

[commit-bot: I haz the power, 2016-04-04 02:13:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-04 02:13:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Nico, 2016-04-04 02:15:39]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez
==========

[Nico, 2016-04-04 02:16:03]

thakis@chromium.org changed reviewers:
+ asvitkine@chromium.org, wez@chromium.org

[Nico, 2016-04-04 02:16:04]

tbr'ing:
asvitkine: components/metrics
wez: remoting

[Nico, 2016-04-04 02:16:13]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2016-04-04 02:16:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1852143002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1852143002/140001

[commit-bot: I haz the power, 2016-04-04 02:21:20]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez
==========

[commit-bot: I haz the power, 2016-04-04 02:21:21]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-04 02:22:50]

Description was changed from

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez
==========

to

==========
win: Remove GetModuleFromAddress, deduplicate __ImageBase code.

8 different places were using __ImageBase to get the current module,
create a CURRENT_MODULE() macro that all these places can use.

All but one use of base::GetModuleFromAddress() were used to get
the module of the current file; change all these to use
CURRENT_MODULE() instead.

The (somewhat lame) motivation is that GetModuleFromAddress() took
a void* but was always called with function pointers, and converting
function pointers to void* has undefined behavior. POSIX (dlsym())
requires that conversions between void* and function pointers work
in practice, but on posix an explicit reinterpret_cast is required, and it
seems nice to have compilers behave identically between platforms
here (i.e. we should turn on clang/win's -Wmicrosoft-cast).

In the one place where GetModuleFromAddress() didn't refer to the
current TU's module, add an explicit reinterpret_cast.

BUG=550065
TBR=asvitkine,wez

Committed: https://crrev.com/d62f5447b35d5d8d1b3521a05b7ed312ab31dbd1
Cr-Commit-Position: refs/heads/master@{#384841}
==========

[commit-bot: I haz the power, 2016-04-04 02:22:51]

Patchset 8 (id:??) landed as
https://crrev.com/d62f5447b35d5d8d1b3521a05b7ed312ab31dbd1
Cr-Commit-Position: refs/heads/master@{#384841}

[sof, 2016-04-04 07:32:43]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-04-04 07:32:44]

https://codereview.chromium.org/1852143002/diff/140001/base/process/memory_un...
File base/process/memory_unittest.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/base/process/memory_un...
base/process/memory_unittest.cc:50: const int kConstantInModule = 42;
This now appears unused --
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds...

[sof, 2016-04-04 07:49:08]

https://codereview.chromium.org/1852143002/diff/140001/base/process/memory_un...
File base/process/memory_unittest.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/base/process/memory_un...
base/process/memory_unittest.cc:50: const int kConstantInModule = 42;
On 2016/04/04 07:32:44, sof wrote:
> This now appears unused --
>
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds...

Removing in https://codereview.chromium.org/1857693002/

[Alexei Svitkine (slow), 2016-04-04 15:16:02]

lgtm

[joedow, 2016-04-04 15:37:00]

joedow@chromium.org changed reviewers:
+ joedow@chromium.org

[joedow, 2016-04-04 15:37:01]

lgtm remoting

https://codereview.chromium.org/1852143002/diff/140001/base/win/current_module.h
File base/win/current_module.h (right):

https://codereview.chromium.org/1852143002/diff/140001/base/win/current_modul...
base/win/current_module.h:1: // Copyright (c) 2016 The Chromium Authors. All
rights reserved.
remove (c):
https://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/1852143002/diff/140001/base/win/current_modul...
base/win/current_module.h:11: // Returs the HMODULE of the dll the macro was
expanded in.
s/Returs/Returns

https://codereview.chromium.org/1852143002/diff/140001/remoting/host/continue...
File remoting/host/continue_window_win.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/remoting/host/continue...
remoting/host/continue_window_win.cc:59: nullptr, (DLGPROC)DialogProc, (LPARAM)
this);
Why the extra space for 'this' and not in front of the other casts?

[Nico, 2016-04-04 15:43:08]

Thanks, addressing most of your comments at
https://codereview.chromium.org/1856683003

https://codereview.chromium.org/1852143002/diff/140001/remoting/host/continue...
File remoting/host/continue_window_win.cc (right):

https://codereview.chromium.org/1852143002/diff/140001/remoting/host/continue...
remoting/host/continue_window_win.cc:59: nullptr, (DLGPROC)DialogProc, (LPARAM)
this);
On 2016/04/04 15:37:01, joedow wrote:
> Why the extra space for 'this' and not in front of the other casts?

Excellent question. clang-format put it there, but that looks like a bug. Filed
https://llvm.org/bugs/show_bug.cgi?id=27198