210608 - build warnings in nsImapMailFolder

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Henrik Gemal]

whenever I build mail&news imap:

c:/mozilla/mozilla/mailnews/imap/src/nsImapMailFolder.cpp: In member function
   `virtual nsresult nsImapMailFolder::Rename(const PRUnichar*,
   nsIMsgWindow*)':
c:/mozilla/mozilla/mailnews/imap/src/nsImapMailFolder.cpp:1302: warning: cast
   to pointer from integer of different size
c:/mozilla/mozilla/mailnews/imap/src/nsImapMailFolder.cpp: In member function
   `virtual nsresult nsImapMailFolder::NormalEndMsgWriteStream(unsigned int,
   int, nsIImapUrl*)':
c:/mozilla/mozilla/mailnews/imap/src/nsImapMailFolder.cpp:3755: warning: unused
   variable `PRBool needMsgID'

Comment 1

[[e-mail]]

Attachment: patch

fixes warnings, removes unnecessary myCString.Truncate() in ctor,
s/-1/kNotFound/

Comment 2

[[e-mail]]

taking

Comment 3

[David :Bienvenu]

I'll try this patch and if it's OK, I'll check it in. Thx for the patch.

Comment 4

[[e-mail]]

wait a sec... I think this patch is also wrong. :/

the format string that uses &m_hierarchyDelimiter needs to be null-terminated,
right? we'll need a strdup there or something.

new patch coming up...

Comment 5

[[e-mail]]

Attachment: patch v2

ugh... so the original code was correct. I put a comment there and used
reinterpret_cast so folks won't be trapped by this in future.

thanks ;)

Comment 6

[[e-mail]]

bienvenu: just a reminder... any chance of r+sr here if you've got a few extra
mins? thx!

Comment 7

[[e-mail]]

Comment on attachment 126458 [details] [diff] [review]
patch v2

rotating request to mscott.

Comment 8

[Scott MacGregor]

Comment on attachment 126458 [details] [diff] [review]
patch v2

two years old, is this patch still valid? If so, re-request but review for build warnings tend to be low on my priority list. (But not two years low!)

Comment 9

[[e-mail]]

-> default owner

Comment 10

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090417 SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/9e06217fc471
 +http://hg.mozilla.org/comm-central/rev/...)

{
nsImapMailFolder.cpp(2331) : warning C4018: '<' : signed/unsigned mismatch
nsImapMailFolder.cpp(2506) : warning C4018: '<' : signed/unsigned mismatch
nsImapMailFolder.cpp(2653) : warning C4018: '>' : signed/unsigned mismatch
nsImapMailFolder.cpp(2564) : warning C4101: 'created' : unreferenced local variable
nsImapMailFolder.cpp(2746) : warning C4018: '<' : signed/unsigned mismatch
nsImapMailFolder.cpp(3820) : warning C4018: '<' : signed/unsigned mismatch
nsImapMailFolder.cpp(6992) : warning C4018: '>=' : signed/unsigned mismatch
}

Comment 11

[Wayne Mery (:wsmwk)]

bienvenu, does this still reproduce?
Gary, does this still apply? (it'd be a miracle)
timeless, you up for taking on another compiler warning?

Comment 12

[David :Bienvenu]

Comment on attachment 126458 [details] [diff] [review]
patch v2

yeah, it's obsolete

Comment 13

[Wayne Mery (:wsmwk)]

do these warnings still occur?

Comment 14

[:aceman]

Attachment: patch v3

Those that Serge posted are valid. I attach a patch to fix them.
I could not fix the:
mailnews/imap/src/nsImapMailFolder.cpp: In member function 'virtual nsresult nsImapMailFolder::Rename(const nsAString_internal&, nsIMsgWindow*)':
mailnews/imap/src/nsImapMailFolder.cpp:1643:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
fro the original report. The solution in the old patch does not work anymore.
Maybe irving knows how to fix that.

Comment 15

[:aceman]

Oh, I messed the requests a bit. But I intentionally requested feedback only meaning if somebody knows how to add the "cast to pointer from integer of different size"-fix to the patch. But if there is no proposal, then the patch can be reviewed as is.

Comment 16

[:Irving Reid (No longer working on Firefox)]

(In reply to :aceman from comment #14)

> I could not fix the:
> mailnews/imap/src/nsImapMailFolder.cpp: In member function 'virtual nsresult
> nsImapMailFolder::Rename(const nsAString_internal&, nsIMsgWindow*)':
> mailnews/imap/src/nsImapMailFolder.cpp:1643:31: warning: cast to pointer
> from integer of different size [-Wint-to-pointer-cast]
> fro the original report. The solution in the old patch does not work anymore.
> Maybe irving knows how to fix that.

That's a medium-ugly bit of code; basically that array contains parameters that are eventually passed to a variable argument list printf()-like function, which expects all of its arguments to be void* sized. The character value being passed would be coerced silently if it was directly in the function call, but the coersion isn't silent (at least on gcc) because we're initializing an array with it instead.

You should be able to silence it with an extra cast to intptr_t, but you should do a try build to make sure it's available on all platforms.

--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -1640,7 +1640,7 @@ NS_IMETHODIMP nsImapMailFolder::Rename (
       {
         const PRUnichar *formatStrings[] =
         {
-           (const PRUnichar*) m_hierarchyDelimiter
+           (const PRUnichar*)(intptr_t)m_hierarchyDelimiter
         };
         nsString alertString;
         rv = bundle->FormatStringFromID(IMAP_SPECIAL_CHAR,

Comment 17

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 673615 [details] [diff] [review]
patch v3

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

The changes in this patch look fine, even if they do make me wish we weren't so inconsistent with signed/unsigned in our code...

Comment 18

[:aceman]

Attachment: patch v4

Comment 19

[:Irving Reid (No longer working on Firefox)]

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=dc6b269ae8ca

Comment 20

[Mozilla RelEng Bot]

Try run for dc6b269ae8ca is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=dc6b269ae8ca
Results (out of 9 total builds):
    warnings: 8
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-dc6b269ae8ca

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/99507befeb52