Closed Bug 956449 Opened 6 years ago Closed 6 years ago

Fix nsRDFXMLDataSource.cpp:430:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable], and mark directory as FAIL_ON_WARNINGS

Categories

(Core Graveyard :: RDF, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Build warning (using GCC 4.8 on linux):
{
rdf/base/src/nsRDFXMLDataSource.cpp: In destructor ‘virtual RDFXMLDataSourceImpl::~RDFXMLDataSourceImpl()’:

rdf/base/src/nsRDFXMLDataSource.cpp:430:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable]
}

This code hasn't been touched in ages, but this happens to be the only build warning in rdf/base/src (at least on my machine), so we might as well squash it to reduce buildwarning-spam and to allow us to mark that directory as warning-free.
Blocks: buildwarning
Per the comment in the patch, this just uses mozilla::unused to suppress the build warning, since we probably can't meaningfully react to it here.

I'm also happy to just drop 'nsresult rv' entirely here. But it seems slightly better to preserve it, so that in the unlikely event that someone is debugging issues in this code, they can more easily inspect it and detect failures in the functions whose return value it captures.
Attachment #8355706 - Flags: review?(l10n)
Green Linux/Mac/Win/Android try run (demonstrating that the directory really is warning-free, or else the FAIL_ON_WARNINGS annotation would turn things red): https://tbpl.mozilla.org/?tree=Try&rev=689111fc40ce
Other me...
Comment on attachment 8355706 [details] [diff] [review]
patch: pass rv to mozilla::unused

Forwarding this review request over to Benjamin, this is stuff I don't know about.
Attachment #8355706 - Flags: review?(l10n) → review?(benjamin)
Comment on attachment 8355706 [details] [diff] [review]
patch: pass rv to mozilla::unused

We don't need rv at all here: just (void) gRDFService->UnregisterDatasource(this) and (void) Flash() to indicate that we're intentionally ignoring the result.
Attachment #8355706 - Flags: review?(benjamin) → review-
Sounds good to me (per end of comment 1). Updated patch coming in a minute.
Attached patch fix v2: drop rv and use (void) (obsolete) — Splinter Review
Updated fix, per comment 5.
Attachment #8356192 - Flags: review?(benjamin)
Attached patch fix v2a: drop rv and use (void) (obsolete) — Splinter Review
...and now with a space between (void) and the function, to better-match the suggestion in comment 5.
Attachment #8356192 - Attachment is obsolete: true
Attachment #8356192 - Flags: review?(benjamin)
Attachment #8356193 - Flags: review?(benjamin)
(sorry, one final tweak, after failing to compile :))

...now actually dropping the decl of 'rv' and restoring a "gRDFService->" that I'd accidentally dropped (triggering a compile error).
Attachment #8356193 - Attachment is obsolete: true
Attachment #8356193 - Flags: review?(benjamin)
Attachment #8356195 - Flags: review?(benjamin)
Attachment #8356195 - Flags: review?(benjamin) → review+
Summary: nsRDFXMLDataSource.cpp:430:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable] → Fix nsRDFXMLDataSource.cpp:430:14: warning: variable ‘rv’ set but not used [-Wunused-but-set-variable], and mark directory as FAIL_ON_WARNINGS
https://hg.mozilla.org/mozilla-central/rev/1e04627cb064
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.