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

Status: RESOLVED FIXED

(Core Graveyard :: RDF, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: patch: pass rv to mozilla::unused

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.

Comment 2

[Daniel Holbert [:dholbert]]

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

Comment 3

[Axel Hecht]

Other me...

Comment 4

[Axel Hecht]

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.

Comment 5

[Benjamin Smedberg]

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.

Comment 6

[Daniel Holbert [:dholbert]]

Sounds good to me (per end of comment 1). Updated patch coming in a minute.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: fix v2: drop rv and use (void)

Updated fix, per comment 5.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: fix v2a: drop rv and use (void)

...and now with a space between (void) and the function, to better-match the suggestion in comment 5.

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: fix v2b: drop rv and use (void)

(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).

Comment 10

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e04627cb064

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1e04627cb064