856552 - Fixed some warnings in xpcom/base/ nsTraceRefcntImpl.cpp and nsStackWalk.cpp

Status: RESOLVED INVALID

(Core :: XPCOM, defect)

Description

[Bindar Robert]

/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘int DumpSerialNumbers(PLHashEntry*, int, void*)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:455:48: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void InitTraceLog()’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:819:43: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogAddRef(void*, nsrefcnt, const char*, uint32_t)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:995:54: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1006:100: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogRelease(void*, nsrefcnt, const char*)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1054:101: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1066:54: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogCtor(void*, const char*, uint32_t)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1106:67: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogDtor(void*, const char*, uint32_t)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1147:67: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogCOMPtrAddRef(void*, nsISupports*)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1188:93: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘intptr_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp: In function ‘void NS_LogCOMPtrRelease(void*, nsISupports*)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsTraceRefcntImpl.cpp:1229:93: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘intptr_t {aka int}’ [-Wformat]

/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp: In function ‘nsresult NS_FormatCodeAddressDetails(void*, const nsCodeAddressDetails*, char*, uint32_t)’:
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1324:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘ptrdiff_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1324:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘ptrdiff_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1328:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘ptrdiff_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1328:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘ptrdiff_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1328:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘ptrdiff_t {aka int}’ [-Wformat]
/home/bindar/Desktop/mozilla-central/xpcom/base/nsStackWalk.cpp:1328:72: warning: format ‘%lX’ expects argument of type ‘long unsigned int’, but argument 7 has type ‘ptrdiff_t {aka int}’ [-Wformat]

Comment 1

[Bindar Robert]

I'm new in development, I will submit a patch soon.

Comment 2

[Bindar Robert]

Attachment: patch v1

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 731793 [details] [diff] [review]
patch v1

Review of attachment 731793 [details] [diff] [review]:
-----------------------------------------------------------------

over to dbaron, sorry for the latency

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 731793 [details] [diff] [review]
patch v1

Casts seem likely to be the wrong solution here; if the format string doesn't match the type of the variable, it seems like the format string should be changed so it does match the type of the variable so that the information printed is correct on all platforms.  (This is especially important for pointers; it's less of an issue for numbers that are unlikely to go above 32 bits.)

And if you do cast, for casts to integers in C++, I prefer construction-style casts rather than static_cast, i.e., int(foo) rather than static_cast<int>(foo).

There are also some wacky whitespace changes, probably tabs.

What did you do to test this patch?  (Fixing warnings like this is the sort of thing I'd expect to come from somebody who is otherwise working on the code in question; it's pretty odd to see this sort of patch come from a new contributor.  And I'm also really not sure what motivated you to patch this.)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That said, if what I said above doesn't make sense, you should feel free to argue/discuss.

Comment 6

[Valentin Gosu [:valentin] (he/him)]

I assume this doesn't apply anymore.