Extracted logging::SystemErrorCodeToString function.

Extracted logging::SystemErrorCodeToString function.
Allows to use any errors, and particularly useful for logging HRESULT on Windows.

Included error code on windows into error message.

Removed unused LOG_GETLASTERROR_MODULE logging macros.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265460

[brettw, 2014-04-21 16:45:07]

People keep trying to add different logging variants and if we added them all,
the file would be unusable. I think we already have too many: most people don't
know what PLOG and friends do at all, meaning we would probably be better off
being explicit in many cases (i.e. replacing LOG_GETLASTERROR with just LOG <<
GetLastError()).

Despite my opinions here, the reason we have so many is that google3 has so many
and we try to be as close as reasonably possible. In this case, google3 doesn't
have these variants, so I'm pretty reluctant to add more.

[Vitaly Buka (NO REVIEWS), 2014-04-21 19:10:59]

How about this one?
Not it's just small cleanup and new function logging::SystemErrorCodeToString.

[Vitaly Buka (NO REVIEWS), 2014-04-21 19:11:20]

How about this one?
Now it's just small cleanup and new function logging::SystemErrorCodeToString.

[grt (UTC plus 2), 2014-04-22 17:02:34]

lgtm. i like that this will give us a handy replacement for the various ways
HRESULTs are logged across the codebase. there's a random smattering of
everything from verbose StringPrintf, pure "<<" (the least useful), and "<<
std::hex <<" here and there.

[willchan no longer on Chromium, 2014-04-22 18:16:00]

Removed myself from reviewers since Brett's already commented earlier :)
Re-add me if needed.


On Tue, Apr 22, 2014 at 10:02 AM, <grt@chromium.org> wrote:

> lgtm. i like that this will give us a handy replacement for the various
> ways
> HRESULTs are logged across the codebase. there's a random smattering of
> everything from verbose StringPrintf, pure "<<" (the least useful), and "<<
> std::hex <<" here and there.
>
> https://codereview.chromium.org/244583002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[brettw, 2014-04-22 19:29:44]

Looks great! LGTM

https://codereview.chromium.org/244583002/diff/130001/base/logging.cc
File base/logging.cc (right):

https://codereview.chromium.org/244583002/diff/130001/base/logging.cc#newcode742
base/logging.cc:742: msgbuf[--len] = 0;
If you happen to know why we do this loop (I realize you only moved it), can you
add a comment? It seems obscure.

[Vitaly Buka (NO REVIEWS), 2014-04-22 21:51:09]

https://codereview.chromium.org/244583002/diff/130001/base/logging.cc
File base/logging.cc (right):

https://codereview.chromium.org/244583002/diff/130001/base/logging.cc#newcode742
base/logging.cc:742: msgbuf[--len] = 0;
On 2014/04/22 19:29:44, brettw wrote:
> If you happen to know why we do this loop (I realize you only moved it), can
you
> add a comment? It seems obscure.

Done.

[Vitaly Buka (NO REVIEWS), 2014-04-22 21:51:12]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-04-22 21:52:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/244583002/180001

[commit-bot: I haz the power, 2014-04-23 01:11:06]

Change committed as 265460