fix warning in third_party/leveldatabase/chromium_logger.h

fix warning in third_party/leveldatabase/chromium_logger.h

Cast thread_id to uint64 to make sure the PRIu64 printf format works.
Otherwise gcc 4.8 gives an error like so:
warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 11 has type ‘long long unsigned int’ [-Wformat=]

Followup to https://codereview.chromium.org/416633002

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

[Mostyn Bramley-Moore, 2014-07-31 20:41:39]

@cmumford, jsbell: does this look OK?

[jsbell, 2014-07-31 20:55:52]

Weird - the format_macros are intended for this very purpose!

What does gcc4.8 think of static_cast to uint64? (Which should be the same as
unsigned long long, though...)

Can you tell if there's a flaw in base/format_macros.h?

[cmumford, 2014-07-31 20:56:13]

lgtm

https://codereview.chromium.org/432083003/diff/1/third_party/leveldatabase/ch...
File third_party/leveldatabase/chromium_logger.h (left):

https://codereview.chromium.org/432083003/diff/1/third_party/leveldatabase/ch...
third_party/leveldatabase/chromium_logger.h:49:
"%04d/%02d/%02d-%02d:%02d:%02d.%03d %" PRIu64 " ",
On some platforms (OSX I believe) pid_t (PlatformThreadId) is a *signed*
integer, and on others it's a long long unsigned. I'm not sure, but this might
get you into trouble. The last time I touched this code I was tempted to convert
this to std::ostringstream.

I think that if this passes the buildbots you'll be OK.

[Mostyn Bramley-Moore, 2014-07-31 21:09:32]

On 2014/07/31 20:55:52, jsbell wrote:
> Weird - the format_macros are intended for this very purpose!
> 
> What does gcc4.8 think of static_cast to uint64? (Which should be the same as
> unsigned long long, though...)

gcc 4.8 doesn't give an error when static casting to uint64 instead.  Woul you
prefer this?

> Can you tell if there's a flaw in base/format_macros.h?

I don't think that's the case in this instance- /usr/include/inttypes.h defines
PRIu64 (ubuntu 14.04), and base/format_macros.h only defines PRIu64 if it isn't
already defined.

[jsbell, 2014-07-31 21:16:13]

On 2014/07/31 21:09:32, Mostyn Bramley-Moore wrote:
> On 2014/07/31 20:55:52, jsbell wrote:
> > Weird - the format_macros are intended for this very purpose!
> > 
> > What does gcc4.8 think of static_cast to uint64? (Which should be the same
as
> > unsigned long long, though...)
> 
> gcc 4.8 doesn't give an error when static casting to uint64 instead.  Woul you
> prefer this?

Yes, since the point of the PRIxx macros are to avoid problems like this across
platforms.

Thanks!

[Mostyn Bramley-Moore, 2014-07-31 21:22:18]

Done: switched to cast to uint64.

https://codereview.chromium.org/432083003/diff/1/third_party/leveldatabase/ch...
File third_party/leveldatabase/chromium_logger.h (left):

https://codereview.chromium.org/432083003/diff/1/third_party/leveldatabase/ch...
third_party/leveldatabase/chromium_logger.h:49:
"%04d/%02d/%02d-%02d:%02d:%02d.%03d %" PRIu64 " ",
On 2014/07/31 20:56:13, cmumford wrote:
> On some platforms (OSX I believe) pid_t (PlatformThreadId) is a *signed*
> integer, and on others it's a long long unsigned. I'm not sure, but this might
> get you into trouble. The last time I touched this code I was tempted to
convert
> this to std::ostringstream.

(I'll switch to jsbell's suggestion.)

> I think that if this passes the buildbots you'll be OK.

The mac_chromium_compile_rel trybot thought it didn't need to recompile with
this patch, I wonder if that's a bug on the trybot?

[jsbell, 2014-07-31 21:35:51]

On 2014/07/31 21:22:18, Mostyn Bramley-Moore wrote:

> The mac_chromium_compile_rel trybot thought it didn't need to recompile with
> this patch, I wonder if that's a bug on the trybot?

Oh, nice observation. chromium_logger.h isn't listed in the leveldatabase.gyp -
can you add it?

[Mostyn Bramley-Moore, 2014-07-31 21:42:52]

> > The mac_chromium_compile_rel trybot thought it didn't need to recompile with
> > this patch, I wonder if that's a bug on the trybot?
> 
> Oh, nice observation. chromium_logger.h isn't listed in the leveldatabase.gyp
-
> can you add it?

Done.  While doing that I noticed this comment: "Include and then exclude so
that all files show up in IDEs, even if they don't build", but a few lines up
there's a condition that adds env_chromium_win.{cc,h} - would you like me to
move that into the main source list (shouldn't the _win.* suffix cause them to
be filtered on the various platforms?).

[jsbell, 2014-07-31 23:06:05]

On 2014/07/31 21:42:52, Mostyn Bramley-Moore wrote:
> Done.  

Thanks!

> While doing that I noticed this comment: "Include and then exclude so
> that all files show up in IDEs, even if they don't build", but a few lines up
> there's a condition that adds env_chromium_win.{cc,h} - would you like me to
> move that into the main source list (shouldn't the _win.* suffix cause them to
> be filtered on the various platforms?).

Yeah, definitely inconsistent. The
['exclude',·'_(android|example|portable|posix)\\.cc$'] line wouldn't filter out
the *_win.{cc,h} files, though. And no .h files would be filtered out. Bleah.

skia/gyp/gpu.gyp or some other files might have better patterns to follow. How
about a separate CL to clean up the gyp?

[jsbell, 2014-07-31 23:06:21]

lgtm

[Mostyn Bramley-Moore, 2014-07-31 23:22:37]

> skia/gyp/gpu.gyp or some other files might have better patterns to follow. How
> about a separate CL to clean up the gyp?

I'll look into it tomorrow (it's late here).

[Mostyn Bramley-Moore, 2014-07-31 23:22:41]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-07-31 23:24:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mostynb@opera.com/432083003/40001

[commit-bot: I haz the power, 2014-08-01 07:49:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-01 14:54:32]

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

[commit-bot: I haz the power, 2014-08-01 14:54:32]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Mostyn Bramley-Moore, 2014-08-01 14:55:47]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-08-01 14:59:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mostynb@opera.com/432083003/40001

[commit-bot: I haz the power, 2014-08-01 21:51:16]

Change committed as 287094