fix build warning in docshell

RESOLVED INVALID

Status

()

RESOLVED INVALID
6 years ago
4 months ago

People

(Reporter: gia, Assigned: gia)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
/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]
(Assignee)

Comment 1

6 years ago
I will submit the patch soon.
(Assignee)

Comment 2

6 years ago
Created attachment 736974 [details]
patch v1
Attachment #736974 - Flags: review?(justin.lebar+bug)
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
Attachment #736974 - Flags: review?(justin.lebar+bug)
(Assignee)

Comment 5

6 years ago
This sounds tricky, but I will try to add the printf type specifiers
(Assignee)

Comment 6

6 years ago
could you assign this bug to me?
Even better, you should now have permission to set it yourself.  :)
(Assignee)

Updated

6 years ago
Assignee: nobody → georgiana.chelu93
Fixed in bug 944694 Patch 1.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.