nsAutoCompleteController.cpp:1070:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

RESOLVED FIXED in mozilla21

Status

()

Toolkit
Autocomplete
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla21
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
{
toolkit/components/autocomplete/nsAutoCompleteController.cpp: In member function ‘void nsAutoCompleteController::AfterSearches()’:
toolkit/components/autocomplete/nsAutoCompleteController.cpp:1070:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

toolkit/components/autocomplete/nsAutoCompleteController.cpp: In member function ‘nsresult nsAutoCompleteController::StartSearches()’:
toolkit/components/autocomplete/nsAutoCompleteController.cpp:1121:30: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
}
(Assignee)

Comment 1

6 years ago
Created attachment 708172 [details] [diff] [review]
fix
Attachment #708172 - Flags: review?(mak77)
(Assignee)

Comment 2

6 years ago
Normally I'd want to add some range-checks before blindly converting an int32_t to a uint32_t, but I don't think it's worth it here, because:

 (a) the contextual code in nsAutoCompleteController.cpp already stores the Count() result in an unsigned value in several places already. (w/out range checks)

 (b) Under the hood, nsCOMArray::Count()'s implementation[1] actually returns a uint32_t, but it just stuffs it into an int32_t (for historical reasons I assume).
 
[1] https://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsCOMArray.h#62
(Assignee)

Updated

6 years ago
Assignee: nobody → dholbert
(Assignee)

Updated

6 years ago
Blocks: 836348
(Assignee)

Comment 3

6 years ago
Created attachment 708190 [details] [diff] [review]
fix v2

oops, prev fix didn't compile due to missing close-paren. Fixed here.
Attachment #708172 - Attachment is obsolete: true
Attachment #708172 - Flags: review?(mak77)
Attachment #708190 - Flags: review?(mak77)
Comment on attachment 708190 [details] [diff] [review]
fix v2

Review of attachment 708190 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with casting made with static_cast<> instead of uint32_t().
Attachment #708190 - Flags: review?(mak77) → review+
(Assignee)

Comment 5

6 years ago
Switched to static_cast and pushed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/5e57a1b8f7b4
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/5e57a1b8f7b4
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Updated

5 years ago
Blocks: 838301
You need to log in before you can comment on or make changes to this bug.