Last Comment Bug 210608 - build warnings in nsImapMailFolder
: build warnings in nsImapMailFolder
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 19.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks: buildwarning 90906
  Show dependency treegraph
 
Reported: 2003-06-25 03:09 PDT by Henrik Gemal
Modified: 2012-10-22 16:55 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.79 KB, patch)
2003-06-25 06:59 PDT, dwitte@gmail.com
no flags Details | Diff | Review
patch v2 (4.04 KB, patch)
2003-06-25 08:04 PDT, dwitte@gmail.com
no flags Details | Diff | Review
patch v3 (12.69 KB, patch)
2012-10-20 13:35 PDT, :aceman
irving: review+
Details | Diff | Review
patch v4 (13.29 KB, patch)
2012-10-22 12:19 PDT, :aceman
acelists: review+
Details | Diff | Review

Description Henrik Gemal 2003-06-25 03:09:51 PDT
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 dwitte@gmail.com 2003-06-25 06:59:22 PDT
Created attachment 126451 [details] [diff] [review]
patch

fixes warnings, removes unnecessary myCString.Truncate() in ctor,
s/-1/kNotFound/
Comment 2 dwitte@gmail.com 2003-06-25 07:02:09 PDT
taking
Comment 3 David :Bienvenu 2003-06-25 07:32:59 PDT
I'll try this patch and if it's OK, I'll check it in. Thx for the patch.
Comment 4 dwitte@gmail.com 2003-06-25 07:37:33 PDT
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 dwitte@gmail.com 2003-06-25 08:04:28 PDT
Created attachment 126458 [details] [diff] [review]
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 dwitte@gmail.com 2003-09-29 23:08:22 PDT
bienvenu: just a reminder... any chance of r+sr here if you've got a few extra
mins? thx!
Comment 7 dwitte@gmail.com 2003-11-25 19:03:21 PST
Comment on attachment 126458 [details] [diff] [review]
patch v2

rotating request to mscott.
Comment 8 Scott MacGregor 2005-11-07 18:39:06 PST
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 dwitte@gmail.com 2007-06-13 22:05:36 PDT
-> default owner
Comment 10 Serge Gautherie (:sgautherie) 2009-04-18 09:10:52 PDT
[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, use Needinfo for questions) 2010-09-01 09:50:46 PDT
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 2010-09-01 10:25:01 PDT
Comment on attachment 126458 [details] [diff] [review]
patch v2

yeah, it's obsolete
Comment 13 Wayne Mery (:wsmwk, use Needinfo for questions) 2012-10-20 10:06:21 PDT
do these warnings still occur?
Comment 14 :aceman 2012-10-20 13:35:34 PDT
Created attachment 673615 [details] [diff] [review]
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 2012-10-22 03:16:04 PDT
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) 2012-10-22 11:12:09 PDT
(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) 2012-10-22 11:39:09 PDT
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 2012-10-22 12:19:24 PDT
Created attachment 673958 [details] [diff] [review]
patch v4
Comment 19 :Irving Reid (No longer working on Firefox) 2012-10-22 13:00:11 PDT
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=dc6b269ae8ca
Comment 20 Mozilla RelEng Bot 2012-10-22 15:00:32 PDT
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/ireid@mozilla.com-dc6b269ae8ca
Comment 21 Ryan VanderMeulen [:RyanVM] 2012-10-22 16:55:28 PDT
https://hg.mozilla.org/comm-central/rev/99507befeb52

Note You need to log in before you can comment on or make changes to this bug.