Note: There are a few cases of duplicates in user autocompletion which are being worked on.

build warnings in nsImapMailFolder

RESOLVED FIXED in Thunderbird 19.0

Status

MailNews Core
Networking: IMAP
--
minor
RESOLVED FIXED
14 years ago
5 years ago

People

(Reporter: Henrik Gemal, Assigned: aceman)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 19.0
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

14 years ago
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

14 years ago
Created attachment 126451 [details] [diff] [review]
patch

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

Updated

14 years ago
Attachment #126451 - Flags: superreview?(bienvenu)
Attachment #126451 - Flags: review?(bienvenu)

Comment 2

14 years ago
taking
Assignee: bienvenu → dwitte

Comment 3

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

Comment 4

14 years ago
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...

Updated

14 years ago
Attachment #126451 - Flags: superreview?(bienvenu)
Attachment #126451 - Flags: review?(bienvenu)

Comment 5

14 years ago
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 ;)
Attachment #126451 - Attachment is obsolete: true

Updated

14 years ago
Attachment #126458 - Flags: superreview?(bienvenu)

Updated

14 years ago
Blocks: 187528

Comment 6

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

Comment 7

14 years ago
Comment on attachment 126458 [details] [diff] [review]
patch v2

rotating request to mscott.
Attachment #126458 - Flags: superreview?(mscott)
Attachment #126458 - Flags: superreview?(bienvenu)
Attachment #126458 - Flags: review?(mscott)
Product: MailNews → Core

Comment 8

12 years ago
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!)
Attachment #126458 - Flags: superreview?(mscott)
Attachment #126458 - Flags: review?(mscott)

Comment 9

10 years ago
-> default owner
Assignee: dwitte → bienvenu
QA Contact: grylchan → networking.imap
Product: Core → MailNews Core
[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
}
Depends on: 90906
Blocks: 90906
No longer depends on: 90906
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

7 years ago
Comment on attachment 126458 [details] [diff] [review]
patch v2

yeah, it's obsolete
Attachment #126458 - Attachment is obsolete: true

Updated

5 years ago
Assignee: dbienvenu → nobody
do these warnings still occur?
Flags: needinfo?(acelists)
(Assignee)

Comment 14

5 years ago
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.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #673615 - Flags: review?
Attachment #673615 - Flags: feedback?(irving)
Flags: needinfo?(acelists)
(Assignee)

Updated

5 years ago
OS: Windows 2000 → All
Hardware: x86 → All
Attachment #673615 - Flags: review?(irving)
Attachment #673615 - Flags: review?
Attachment #673615 - Flags: feedback?(irving)
(Assignee)

Comment 15

5 years ago
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.
(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 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...
Attachment #673615 - Flags: review?(irving) → review+
(Assignee)

Comment 18

5 years ago
Created attachment 673958 [details] [diff] [review]
patch v4
Attachment #673615 - Attachment is obsolete: true
Attachment #673958 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [please run a try-build on all platforms before checkin, per comment 16]
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=dc6b269ae8ca

Comment 20

5 years ago
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
https://hg.mozilla.org/comm-central/rev/99507befeb52
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [please run a try-build on all platforms before checkin, per comment 16]
Target Milestone: --- → Thunderbird 19.0
You need to log in before you can comment on or make changes to this bug.