Closed Bug 717015 Opened 8 years ago Closed 8 years ago

nsGenericHTMLElement.cpp:1635:12: error: unused variable ‘rv’ [-Werror=unused-variable]

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 717004

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 716338 turned on warnings-as-errors in content/html/content/src/Makefile.in

This breaks my local build (with --enable-warnings-as-errors), due to this build warning:
{
nsGenericHTMLElement.cpp:1635:12: error: unused variable ‘rv’ [-Werror=unused-variable]
}
This is for this line:
> 1622 nsGenericHTMLElement::RestoreFormControlState(nsGenericHTMLElement* aContent,
> 1623                                               nsIFormControl* aControl)
> 1624 {
[...]
> 1635   rv = history->GetState(key, &state);
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsGenericHTMLElement.cpp#1635
The return value from this call has been ignored since this line of code was added, for bug 77834:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/content/html/content/src&command=DIFF_FRAMESET&file=nsGenericHTMLElement.cpp&rev2=1.343&rev1=1.342

We check whether |state| is null on the next line -- it looks like that's used as an indicator of whether the GetState() call failed.  (Hopefully that's a fair assumption?)
Actually, nsLayoutHistoryState::GetState only ever returns NS_OK.

That's the only impl of the nsILayoutHistoryState interface that I can find...
http://mxr.mozilla.org/mozilla-central/search?string=public+nsILayoutHistoryState&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
...so unless we expect extensions to provide additional impls for this interface, we can safely just assume that this method will always succeed.
On the other hand, maybe it's better to be hygenic and check for success before dereferencing the potentially-uninitialized-value |state|.  (we actually know it will be initialized, but that's because we're looking across the interface boundary)
Attachment #587458 - Attachment description: fix → fix: check rv before using returned-by-reference |state|
I only just noticed this is an extension of bug 717004's issues. :)  (I was in "try to build; hit build error; file bug w/ patch; repeat " mode when I filed this.)

uping to that bug; posted a merged patch there.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 717004
Attachment #587458 - Flags: review?(jst)
You need to log in before you can comment on or make changes to this bug.