Closed Bug 830439 Opened 12 years ago Closed 12 years ago

nsWebBrowserFind.cpp:501:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]

Categories

(Core Graveyard :: Embedding: APIs, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla21

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Build warning: { embedding/components/find/src/nsWebBrowserFind.cpp: In member function ‘nsresult nsWebBrowserFind::GetSearchLimits(nsIDOMRange*, nsIDOMRange*, nsIDOMRange*, nsIDOMDocument*, nsISelection*, bool)’: embedding/components/find/src/nsWebBrowserFind.cpp: variable ‘rv’ set but not used embedding/components/find/src/nsWebBrowserFind.cpp:501:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable] }
Attached patch fix v1 (obsolete) (deleted) — Splinter Review
This patch adds NS_ENSURE_SUCCESS(rv,rv) for the two places where we currently just ignore |rv|. The first method (GetRangeCount) shouldn't ever fail, because it only has 1 impl, which always returns NS_OK: https://mxr.mozilla.org/mozilla-central/source/layout/generic/nsSelection.cpp#4646 The second method (GetRootNode) can fail, but we already check it for failure implicitly via NS_ENSURE_ARG_POINTER() on something derived from its outparam. The NS_ENSURE_SUCCESS in this patch just makes us return slightly earlier, if it does fail (and be more hygenic about not touching outparams from a failed function-call). Patch marks this directory as warning-free, too. It's warning-free on my local machine, at least -- seeing how that holds up on other platforms: https://tbpl.mozilla.org/?tree=Try&rev=a085e9116975
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #701909 - Flags: review?(matspal)
Attached patch fix v1a (no FAIL_ON_WARNINGS) (deleted) — Splinter Review
That TBPL run was red on Android & Mac due to other warnings (with the Mac warnings being nontrivial to fix -- something about deprecated functions), so I'm dropping the FAIL_ON_WARNINGS annotation.
Attachment #701909 - Attachment is obsolete: true
Attachment #701909 - Flags: review?(matspal)
Attachment #702014 - Flags: review?(matspal)
Comment on attachment 702014 [details] [diff] [review] fix v1a (no FAIL_ON_WARNINGS) r=mats without the FAIL_ON_WARNINGS (you probably uploaded the old patch)
Attachment #702014 - Flags: review?(matspal) → review+
Yup, I must've forgotten to qref or something. Sorry about that, & thanks for the review! https://hg.mozilla.org/integration/mozilla-inbound/rev/eb448d8799da
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Blocks: 837903
(In reply to Daniel Holbert [:dholbert] from comment #3) > That TBPL run was red on Android & Mac due to other warnings (with the Mac > warnings being nontrivial to fix -- something about deprecated functions), > so I'm dropping the FAIL_ON_WARNINGS annotation. (Note: we no let deprecated-function build warnings run afoul of FAIL_ON_WARNINGS, and I fixed the other build warning, so we can annotate this directory now. I filed bug 837903 on that.)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: