Closed Bug 1003921 Opened 10 years ago Closed 10 years ago

storage/src/mozStorageAsyncStatementExecution.cpp:456:18: warning: ignoring return value of function declared with const attribute [-Wunused-value]

Categories

(Toolkit :: Storage, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox32 --- affected

People

(Reporter: cpeterson, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I tried to FAIL_ON_WARNINGS storage/src:

https://tbpl.mozilla.org/?tree=Try&rev=bd6c050df6df

and the B2G Desktop OS X Opt build warngs (using gcc 4.4, I believe):

storage/src/mozStorageAsyncStatementExecution.cpp:456:18: error: ignoring return value of function declared with const attribute [-Werror,-Wunused-value]

NS_WARN_IF(NS_FAILED(mConnection->rollbackTransactionInternal(mNativeConnection)));
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../dist/include/nsError.h:183:40: note: expanded from macro 'NS_FAILED'
#define NS_FAILED(_nsresult)    ((bool)MOZ_UNLIKELY(NS_FAILED_impl(_nsresult)))
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../dist/include/mozilla/Likely.h:17:28: note: expanded from macro 'MOZ_UNLIKELY'
#  define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
                           ^~~~~~~~~~~~~~~~ ~~~~~~~~
../../dist/include/nsDebug.h:47:38: note: expanded from macro 'NS_WARN_IF'
#define NS_WARN_IF(condition) (bool)(condition)
                                     ^
(In reply to Chris Peterson (:cpeterson) from comment #0)
> and the B2G Desktop OS X Opt build warngs (using gcc 4.4, I believe):

I don't think we build with --enable-warnings-as-errors for any gcc 4.4 builds; bug 915555 makes that impossible currently.

IIRC we (have to) use GCC 4.4 to build B2G for devices, but we don't for desktop emulator builds.
In any case; we want to be using NS_WARN_IF_FALSE() here.

NS_WARN_IF is intended to be used inside of an "if" condition, like:
 if (NS_WARN_IF(NS_FAILED(rv))) {
   return rv;
 }
(In reply to Daniel Holbert [:dholbert] from comment #2)
> In any case; we want to be using NS_WARN_IF_FALSE() here.

er, scratch that; looks like the thing we're checking is something that we want to evaluate in opt builds. So NS_WARN_IF() is sort of what we want; we probably just want to pass it to unused, or something like that, based on the previous state of this code (which used (void)):
http://hg.mozilla.org/mozilla-central/diff/dbf72e1458ff/storage/src/mozStorageAsyncStatementExecution.cpp#l1.63
Blocks: 914070
I think I'd be fine with assigning to rv and NS_WARN_IF_FALSE(NS_SUCCEEDED(rv))
hm, though then we need DebugOnly rv... grr.
Yeah, I agree with comment 4 & 5. I think DebugOnly is fine here. Have WIP patch that does that.
OS: Gonk (Firefox OS) → Mac OS X
Attached patch fix v1Splinter Review
Here's a patch.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=aab159232e39
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8415432 - Flags: review?(mak77)
Try run is green. (Not visible on TBPL yet, but self-serve link already has the results).
Version: unspecified → Trunk
Attachment #8415432 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/5625f2879911
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.