861386 - fix build warning in docshell

Status: RESOLVED INVALID

(Core :: DOM: Navigation, defect)

Description

[Georgiana [:gia]]

/home/gia/mozilla-central/docshell/base/nsDocShell.cpp:801:44: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘uint64_t {aka long unsigned int}’ [-Wformat]
/home/gia/mozilla-central/docshell/base/nsDocShell.cpp: In destructor ‘virtual nsDocShell::~nsDocShell()’:
/home/gia/mozilla-central/docshell/base/nsDocShell.cpp:830:46: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘uint64_t {aka long unsigned int}’ [-Wformat]

Comment 1

[Georgiana [:gia]]

I will submit the patch soon.

Comment 2

[Georgiana [:gia]]

Attachment: patch v1

Comment 3

[Justin Lebar (not reading bugmail)]

It's unfortunately more complicated than this.

On Linux 64 and Mac 64, uint64_t == unsigned long int.  But on Windows 64, uint64_t == unsigned long long int.  So this change just moves the warning from one platform to another.

What you actually want is the PRIu64 macro from inttypes.h [1], which you'd use as

  uint64_t foo;
  printf("foo = " PRIu64 "\n", foo);

but I'm not sure if we have this on all platforms.  I suspect we may not have it on Windows because inttypes.h includes stdint.h, which we had to re-implement ourselves for Windows in mfbt/StandardInteger.h.

If you wanted to add the printf type specifiers from inttypes.h into MSStdInt.h (or an auxiliary header), that would be interesting.  But that may be more complex a patch than you intended to write.  :)

I suppose we could get rid of the warnings by static_cast'ing everything to <long long unsigned> and then using %llu.  That's kind of ugly, but maybe it's better than the warning.

[1] http://pubs.opengroup.org/onlinepubs/009604599/basedefs/inttypes.h.html

Comment 4

[Justin Lebar (not reading bugmail)]

See also http://en.wikipedia.org/wiki/64-bit#64-bit_data_models

Comment 5

[Georgiana [:gia]]

This sounds tricky, but I will try to add the printf type specifiers

Comment 6

[Georgiana [:gia]]

could you assign this bug to me?

Comment 7

[Justin Lebar (not reading bugmail)]

Even better, you should now have permission to set it yourself.  :)

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Fixed in bug 944694 Patch 1.