The default bug view has changed. See this FAQ.

fix compiler warnings in mailnews/base/src/nsMsgDBView.cpp

RESOLVED FIXED in Thunderbird 14.0

Status

MailNews Core
Database
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 14.0
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

6.67 KB, patch
aceman
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Some of the warning seem to be real problems:

mailnews/base/src/nsMsgDBView.cpp: In member function 'nsresult nsMsgDBView::FetchRowKeywords(nsMsgViewIndex, nsIMsgDBHdr*, nsACString_internal&)':
mailnews/base/src/nsMsgDBView.cpp:932:38: warning: comparison between signed and unsigned integer expressions
mailnews/base/src/nsMsgDBView.cpp: In member function 'void nsMsgDBView::RememberDeletedMsgHdr(nsIMsgDBHdr*)':
mailnews/base/src/nsMsgDBView.cpp:1945:65: warning: operation on '((nsMsgDBView*)this)->nsMsgDBView::mRecentlyDeletedArrayIndex' may be undefined
mailnews/base/src/nsMsgDBView.cpp: In member function 'virtual nsMsgViewIndex nsMsgDBView::FindKey(nsMsgKey, bool)':
mailnews/base/src/nsMsgDBView.cpp:4833:33: warning: suggest parentheses around '&&' within '||'
mailnews/base/src/nsMsgDBView.cpp: In member function 'virtual nsresult nsMsgDBView::GetMsgToSelectAfterDelete(nsMsgViewIndex*)':
mailnews/base/src/nsMsgDBView.cpp:7096:11: warning: 'startFirstRange' may be used uninitialized in this function
mailnews/base/src/nsMsgDBView.cpp: In member function 'nsresult nsMsgDBView::GetCollationKey(nsIMsgDBHdr*, nsMsgViewSortTypeValue, PRUint8**, PRUint32*, nsIMsgCustomColumnHandler*)':
mailnews/base/src/nsMsgDBView.cpp:4174:12: warning: 'rv' may be used uninitialized in this function
(Assignee)

Comment 1

5 years ago
Created attachment 603364 [details] [diff] [review]
patch

Please check I do not change the logic somewhere. xpcshell tests pass normally.
Attachment #603364 - Flags: review?(dbienvenu)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED

Comment 2

5 years ago
Comment on attachment 603364 [details] [diff] [review]
patch

this should be reformatted so that the lines are 80 chars or less, the || is at the end of the line, and second line of the if is indented under the first (, not lining up with the retIndex line.

-        if ((flags & nsMsgMessageFlags::Elided) && NS_SUCCEEDED(ExpandByIndex(threadIndex, nsnull))
+        if (((flags & nsMsgMessageFlags::Elided) && NS_SUCCEEDED(ExpandByIndex(threadIndex, nsnull)))
           || (flags & MSG_VIEW_FLAG_DUMMY))
           retIndex = (nsMsgViewIndex) m_keys.IndexOf(key, threadIndex + 1);

I'll look at the logic of it in a bit...
(Assignee)

Updated

5 years ago
Blocks: 187528

Comment 3

5 years ago
Comment on attachment 603364 [details] [diff] [review]
patch

        NS_ASSERTION(false,"should not be here (Sort Type: byCustom (String), but no custom handler)");
-

should be NS_ERROR("should...")

other than that, it looks ok, though I need to double check this change:

+        if (((flags & nsMsgMessageFlags::Elided) && NS_SUCCEEDED(ExpandByIndex(threadIndex, nsnull)))
           || (flags & MSG_VIEW_FLAG_DUMMY))

Comment 4

5 years ago
Comment on attachment 603364 [details] [diff] [review]
patch

thx for the patch
Attachment #603364 - Flags: review?(dbienvenu) → review+
(Assignee)

Comment 5

5 years ago
I assume I still have to fix the nits in comment 2 and 3?

Comment 6

5 years ago
(In reply to :aceman from comment #5)
> I assume I still have to fix the nits in comment 2 and 3?

yes, please, thx.
(Assignee)

Comment 7

5 years ago
Created attachment 610578 [details] [diff] [review]
patch v2

Thanks, r=bienvenu.
Attachment #603364 - Attachment is obsolete: true
Attachment #610578 - Flags: review+
(Assignee)

Updated

5 years ago
Severity: minor → trivial
Keywords: checkin-needed
http://hg.mozilla.org/comm-central/rev/007dc7159f3a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
You need to log in before you can comment on or make changes to this bug.