win: Add (currently unused) NTSTATUS_LOG

win: Add (currently unused) NTSTATUS_LOG

Split out of https://codereview.chromium.org/1336823002/, I'll go and
adapt various places to use it as I need to touch them.

R=mark@chromium.org
BUG=crashpad:1, crashpad:50
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/bc55c7916e96c423c733c73f658b86564e63b097

[Mark Mentovai, 2015-09-15 02:22:32]

LGTM

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.cc
File util/win/ntstatus_logging.cc (right):

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:17: #include <string.h>
<string>, no .h

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:25: DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS |
Optional: const, or write inline in the FormatMessageA()?

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:33: NULL);
nullptr

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:37: return base::StringPrintf("<failed to retrieve
error message (0x%x)>",
base/strings/stringprintf.h

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:51: : LogMessage(function, file_path, line,
severity), ntstatus_(ntstatus) {
:(

mini_chromium has a function argument and Chromium doesn’t.

This will need to be an ugly thing like

#if defined(MINI_CHROMIUM_BASE_LOGGING_H_)
    function,
#endif

See https://user.git/mmentovai/bpoop/+/5032c36203f5/util/mach/mach_logging.cc
and .h. (Sorry for the internal link. This was the previous experimental-esque
repository from when Crashpad was called bpoop and was just my pet project on
the side. mach_logging originated in bpoop and I promoted it to base before
Crashpad became a real project.)

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:56: << base::StringPrintf(" (0x%04x)", ntstatus_);
According to https://msdn.microsoft.com/en-us/library/cc704588.aspx, NTSTATUS
uses all 32 bits, so 0x%08x?

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.h
File util/win/ntstatus_logging.h (right):

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.h...
util/win/ntstatus_logging.h:36: DISALLOW_COPY_AND_ASSIGN(NtstatusLogMessage);
#include "base/basictypes.h".

[scottmg, 2015-09-15 04:33:12]

Patchset #2 (id:20001) has been deleted

[scottmg, 2015-09-15 04:33:51]

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.cc
File util/win/ntstatus_logging.cc (right):

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:17: #include <string.h>
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> <string>, no .h

Done.

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:25: DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS |
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> Optional: const, or write inline in the FormatMessageA()?

Done.

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:33: NULL);
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> nullptr

Done.

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:37: return base::StringPrintf("<failed to retrieve
error message (0x%x)>",
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> base/strings/stringprintf.h

Already there?

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:51: : LogMessage(function, file_path, line,
severity), ntstatus_(ntstatus) {
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> :(
> 
> mini_chromium has a function argument and Chromium doesn’t.
> 
> This will need to be an ugly thing like
> 
> #if defined(MINI_CHROMIUM_BASE_LOGGING_H_)
>     function,
> #endif
> 
> See https://user.git/mmentovai/bpoop/+/5032c36203f5/util/mach/mach_logging.cc
> and .h. (Sorry for the internal link. This was the previous experimental-esque
> repository from when Crashpad was called bpoop and was just my pet project on
> the side. mach_logging originated in bpoop and I promoted it to base before
> Crashpad became a real project.)

Do you find the function in the log particularly useful? Maybe we should just
remove it for consistency.

I don't have access to that link I don't think, but done as suggested anyway.

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:56: << base::StringPrintf(" (0x%04x)", ntstatus_);
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> According to https://msdn.microsoft.com/en-us/library/cc704588.aspx, NTSTATUS
> uses all 32 bits, so 0x%08x?

Done. (No idea what I was thinking there... 4 bytes maybe?)

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.h
File util/win/ntstatus_logging.h (right):

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.h...
util/win/ntstatus_logging.h:36: DISALLOW_COPY_AND_ASSIGN(NtstatusLogMessage);
On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> #include "base/basictypes.h".

Done.

[Mark Mentovai, 2015-09-15 12:52:57]

LGTM

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.cc
File util/win/ntstatus_logging.cc (right):

https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
util/win/ntstatus_logging.cc:51: : LogMessage(function, file_path, line,
severity), ntstatus_(ntstatus) {
scottmg wrote:
> On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> > :(
> > 
> > mini_chromium has a function argument and Chromium doesn’t.
> > 
> > This will need to be an ugly thing like
> > 
> > #if defined(MINI_CHROMIUM_BASE_LOGGING_H_)
> >     function,
> > #endif
> > 
> > See
https://user.git/mmentovai/bpoop/+/5032c36203f5/util/mach/mach_logging.cc
> > and .h. (Sorry for the internal link. This was the previous
experimental-esque
> > repository from when Crashpad was called bpoop and was just my pet project
on
> > the side. mach_logging originated in bpoop and I promoted it to base before
> > Crashpad became a real project.)
> 
> Do you find the function in the log particularly useful?

I guess not. I used to use it, but not anymore. We can get rid of it, and
reintroduce it if we ever want it again.

What I liked about it was that in cases where things got inlined pretty deeply,
__FUNC__ would tell you the actual file that __LINE__ referred to, where
__FILE__ would always just tell you the translation unit.

[scottmg, 2015-09-15 16:22:20]

On 2015/09/15 12:52:57, Mark Mentovai - August is over wrote:
> LGTM
> 
> https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.cc
> File util/win/ntstatus_logging.cc (right):
> 
>
https://codereview.chromium.org/1343873003/diff/1/util/win/ntstatus_logging.c...
> util/win/ntstatus_logging.cc:51: : LogMessage(function, file_path, line,
> severity), ntstatus_(ntstatus) {
> scottmg wrote:
> > On 2015/09/15 02:22:32, Mark Mentovai - August is over wrote:
> > > :(
> > > 
> > > mini_chromium has a function argument and Chromium doesn’t.
> > > 
> > > This will need to be an ugly thing like
> > > 
> > > #if defined(MINI_CHROMIUM_BASE_LOGGING_H_)
> > >     function,
> > > #endif
> > > 
> > > See
> https://user.git/mmentovai/bpoop/+/5032c36203f5/util/mach/mach_logging.cc
> > > and .h. (Sorry for the internal link. This was the previous
> experimental-esque
> > > repository from when Crashpad was called bpoop and was just my pet project
> on
> > > the side. mach_logging originated in bpoop and I promoted it to base
before
> > > Crashpad became a real project.)
> > 
> > Do you find the function in the log particularly useful?
> 
> I guess not. I used to use it, but not anymore. We can get rid of it, and
> reintroduce it if we ever want it again.
> 
> What I liked about it was that in cases where things got inlined pretty
deeply,
> __FUNC__ would tell you the actual file that __LINE__ referred to, where
> __FILE__ would always just tell you the translation unit.

OK. I'll leave it for now as-is. We could also decide to add it in Chromium
instead, but I guess there might be a decent size impact on builds where logging
is turned on.

[scottmg, 2015-09-15 16:22:36]

Committed patchset #2 (id:40001) manually as
bc55c7916e96c423c733c73f658b86564e63b097 (presubmit successful).