Closed
Bug 824296
Opened 12 years ago
Closed 12 years ago
fix some signed/unsigned comparison problems and compile warnings in nsMsgFilterList.cpp
Categories
(MailNews Core :: Filters, defect)
MailNews Core
Filters
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 20.0
People
(Reporter: aceman, Assigned: aceman)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
E.g.:
mailnews/base/search/src/nsMsgFilterList.cpp: In member function 'nsresult nsMsgFilterList::SaveTextFilters(nsIOutputStream*)':
mailnews/base/search/src/nsMsgFilterList.cpp:871:15: warning: variable 'attribStr' set but not used [-Wunused-but-set-variable]
mailnews/base/search/src/nsMsgFilterList.cpp: In member function 'virtual nsresult nsMsgFilterList::MoveFilter(nsIMsgFilter*, nsMsgFilterMotionValue)':
mailnews/base/search/src/nsMsgFilterList.cpp:1043:60: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
From bug 821743: neil@parkwaycc.co.uk 2012-12-22 21:51:03 CET
Comment on attachment 695244 [details] [diff] [review] [diff] [review]
patch
>+ NS_ENSURE_ARG_MAX(aIndex, m_actionList.Length() - 1);
Not sure that this does the right thing for an empty list.
-----------------
The same problem happens here in nsIMsgFilterList.cpp.
Attachment #695261 -
Flags: review?(neil)
Summary: fix some signed/usigned comparison problems and compile warnings in nsMsgFilterList.cpp → fix some signed/unsigned comparison problems and compile warnings in nsMsgFilterList.cpp
Comment 2•12 years ago
|
||
Comment on attachment 695261 [details] [diff] [review]
patch
> nsresult nsMsgFilterList::GetFilterAt(uint32_t filterIndex, nsIMsgFilter **filter)
> {
> NS_ENSURE_ARG_POINTER(filter);
>-
>- uint32_t filterCount = 0;
>- GetFilterCount(&filterCount);
>- NS_ENSURE_ARG_MAX(filterIndex, filterCount - 1);
>+ NS_ENSURE_ARG(filterIndex < m_filters.Length());
You're trying to fix too many bugs at once and ending up with a mishmash. Although it's a good idea to replace calls to GetFilterCount with direct access to Length() you should file a separate bugs and fix up all the cases at once. Please put this back for now so that it looks like the check in MoveFilterAt below. r=me with that fixed.
Attachment #695261 -
Flags: review?(neil) → review+
I do not want to replace any other occurrence of GetFilterCount in that file. If you see the file history I am the one who put them there originally ;) I just wanted to replace this single one. So I will not file any separate bug for that.
So if you consider the change to be good as you say, either accept it here, or I take it out as you wish, but then it will not be done later (at least not by me).
Flags: needinfo?(neil)
Comment 4•12 years ago
|
||
When I said "calls" I meant "all the internal calls". I didn't mean "one call because I felt like it". I can't see any justification for only changing one call.
Flags: needinfo?(neil)
As you wish.
Attachment #695261 -
Attachment is obsolete: true
Attachment #695659 -
Flags: review+
Keywords: checkin-needed
Comment 6•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
You need to log in
before you can comment on or make changes to this bug.
Description
•