Last Comment Bug 733448 - fix compiler warnings in mailnews/base/src/nsMsgDBView.cpp
: fix compiler warnings in mailnews/base/src/nsMsgDBView.cpp
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Database (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: Thunderbird 14.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2012-03-06 10:35 PST by :aceman
Modified: 2012-03-29 14:18 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (5.40 KB, patch)
2012-03-06 11:10 PST, :aceman
mozilla: review+
Details | Diff | Splinter Review
patch v2 (6.67 KB, patch)
2012-03-29 09:56 PDT, :aceman
acelists: review+
Details | Diff | Splinter Review

Description :aceman 2012-03-06 10:35:09 PST
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
Comment 1 :aceman 2012-03-06 11:10:22 PST
Created attachment 603364 [details] [diff] [review]
patch

Please check I do not change the logic somewhere. xpcshell tests pass normally.
Comment 2 David :Bienvenu 2012-03-08 09:56:29 PST
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...
Comment 3 David :Bienvenu 2012-03-28 14:00:01 PDT
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 David :Bienvenu 2012-03-29 09:28:58 PDT
Comment on attachment 603364 [details] [diff] [review]
patch

thx for the patch
Comment 5 :aceman 2012-03-29 09:31:49 PDT
I assume I still have to fix the nits in comment 2 and 3?
Comment 6 David :Bienvenu 2012-03-29 09:34:00 PDT
(In reply to :aceman from comment #5)
> I assume I still have to fix the nits in comment 2 and 3?

yes, please, thx.
Comment 7 :aceman 2012-03-29 09:56:50 PDT
Created attachment 610578 [details] [diff] [review]
patch v2

Thanks, r=bienvenu.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-03-29 14:18:11 PDT
http://hg.mozilla.org/comm-central/rev/007dc7159f3a

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