Skip to content

Commit

Permalink
Bug 1299384 - Use MOZ_MUST_USE with NS_warn_if_impl(). r=erahm.
Browse files Browse the repository at this point in the history
This change avoids lots of false positives for Coverity's CHECKED_RETURN
warning, caused by NS_WARN_IF's current use in both statement-style and
expression-style.

In the case where the code within the NS_WARN_IF has side-effects, I made the
following change.

> NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));
> -->
> Unused << NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));

In the case where the code within the NS_WARN_IF lacks side-effects, I made the
following change.

> NS_WARN_IF(!condWithoutSideEffects);
> -->
> NS_WARNING_ASSERTION(condWithoutSideEffects, "msg");

This has two improvements.
- The condition is not evaluated in non-debug builds.
- The sense of the condition is inverted to the familiar "this condition should
  be true" sense used in assertions.

A common variation on the side-effect-free case is the following.

> nsresult rv = Fn();
> NS_WARN_IF_(NS_FAILED(rv));
> -->
> DebugOnly<nsresult rv> = Fn();
> NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Fn failed");
  • Loading branch information
nnethercote committed Sep 2, 2016
1 parent dc3de1a commit d375269
Show file tree
Hide file tree
Showing 98 changed files with 464 additions and 378 deletions.
8 changes: 5 additions & 3 deletions devtools/shared/heapsnapshot/AutoMemMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "mozilla/devtools/AutoMemMap.h"

#include "mozilla/Unused.h"
#include "nsDebug.h"

namespace mozilla {
Expand All @@ -13,17 +15,17 @@ namespace devtools {
AutoMemMap::~AutoMemMap()
{
if (addr) {
NS_WARN_IF(PR_MemUnmap(addr, size()) != PR_SUCCESS);
Unused << NS_WARN_IF(PR_MemUnmap(addr, size()) != PR_SUCCESS);
addr = nullptr;
}

if (fileMap) {
NS_WARN_IF(PR_CloseFileMap(fileMap) != PR_SUCCESS);
Unused << NS_WARN_IF(PR_CloseFileMap(fileMap) != PR_SUCCESS);
fileMap = nullptr;
}

if (fd) {
NS_WARN_IF(PR_Close(fd) != PR_SUCCESS);
Unused << NS_WARN_IF(PR_Close(fd) != PR_SUCCESS);
fd = nullptr;
}
}
Expand Down
3 changes: 2 additions & 1 deletion devtools/shared/heapsnapshot/HeapSnapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "mozilla/dom/HeapSnapshotBinding.h"
#include "mozilla/RangedPtr.h"
#include "mozilla/Telemetry.h"
#include "mozilla/Unused.h"

#include "jsapi.h"
#include "jsfriendapi.h"
Expand Down Expand Up @@ -1473,7 +1474,7 @@ class DeleteHeapSnapshotTempFileHelperChild
constexpr DeleteHeapSnapshotTempFileHelperChild() { }

void operator()(PHeapSnapshotTempFileHelperChild* ptr) const {
NS_WARN_IF(!HeapSnapshotTempFileHelperChild::Send__delete__(ptr));
Unused << NS_WARN_IF(!HeapSnapshotTempFileHelperChild::Send__delete__(ptr));
}
};

Expand Down
3 changes: 2 additions & 1 deletion devtools/shared/heapsnapshot/ZeroCopyNSIOutputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "mozilla/devtools/ZeroCopyNSIOutputStream.h"

#include "mozilla/DebugOnly.h"
#include "mozilla/Unused.h"

namespace mozilla {
namespace devtools {
Expand All @@ -24,7 +25,7 @@ ZeroCopyNSIOutputStream::ZeroCopyNSIOutputStream(nsCOMPtr<nsIOutputStream>& out)
ZeroCopyNSIOutputStream::~ZeroCopyNSIOutputStream()
{
if (!failed())
NS_WARN_IF(NS_FAILED(writeBuffer()));
Unused << NS_WARN_IF(NS_FAILED(writeBuffer()));
}

nsresult
Expand Down
8 changes: 4 additions & 4 deletions dom/audiochannel/AudioChannelService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,8 @@ AudioChannelService::AudioChannelWindow::NotifyAudioAudibleChanged(nsPIDOMWindow
{
RefPtr<AudioPlaybackRunnable> runnable =
new AudioPlaybackRunnable(aWindow, aAudible, aReason);
nsresult rv = NS_DispatchToCurrentThread(runnable);
NS_WARN_IF(NS_FAILED(rv));
DebugOnly<nsresult> rv = NS_DispatchToCurrentThread(runnable);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "NS_DispatchToCurrentThread failed");
}

void
Expand All @@ -1360,6 +1360,6 @@ AudioChannelService::AudioChannelWindow::NotifyChannelActive(uint64_t aWindowID,
{
RefPtr<NotifyChannelActiveRunnable> runnable =
new NotifyChannelActiveRunnable(aWindowID, aChannel, aActive);
nsresult rv = NS_DispatchToCurrentThread(runnable);
NS_WARN_IF(NS_FAILED(rv));
DebugOnly<nsresult> rv = NS_DispatchToCurrentThread(runnable);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "NS_DispatchToCurrentThread failed");
}
8 changes: 4 additions & 4 deletions dom/base/MultipartBlobImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void
MultipartBlobImpl::InitializeBlob(ErrorResult& aRv)
{
SetLengthAndModifiedDate(aRv);
NS_WARN_IF(aRv.Failed());
NS_WARNING_ASSERTION(!aRv.Failed(), "SetLengthAndModifiedDate failed");
}

void
Expand Down Expand Up @@ -221,7 +221,7 @@ MultipartBlobImpl::InitializeBlob(JSContext* aCx,

mBlobImpls = blobSet.GetBlobImpls();
SetLengthAndModifiedDate(aRv);
NS_WARN_IF(aRv.Failed());
NS_WARNING_ASSERTION(!aRv.Failed(), "SetLengthAndModifiedDate failed");
}

void
Expand Down Expand Up @@ -352,7 +352,7 @@ MultipartBlobImpl::InitializeChromeFile(Blob& aBlob,
mBlobImpls = blobSet.GetBlobImpls();

SetLengthAndModifiedDate(aRv);
NS_WARN_IF(aRv.Failed());
NS_WARNING_ASSERTION(!aRv.Failed(), "SetLengthAndModifiedDate failed");
}

void
Expand Down Expand Up @@ -424,7 +424,7 @@ MultipartBlobImpl::InitializeChromeFile(nsPIDOMWindowInner* aWindow,
mBlobImpls = blobSet.GetBlobImpls();

SetLengthAndModifiedDate(aRv);
NS_WARN_IF(aRv.Failed());
NS_WARNING_ASSERTION(!aRv.Failed(), "SetLengthAndModifiedDate failed");
}

void
Expand Down
24 changes: 13 additions & 11 deletions dom/base/ScreenOrientation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,11 @@ ScreenOrientation::UnlockDeviceOrientation()
// Remove event listener in case of fullscreen lock.
nsCOMPtr<EventTarget> target = do_QueryInterface(GetOwner()->GetDoc());
if (target) {
nsresult rv = target->RemoveSystemEventListener(NS_LITERAL_STRING("fullscreenchange"),
mFullScreenListener, /* useCapture */ true);
NS_WARN_IF(NS_FAILED(rv));
DebugOnly<nsresult> rv =
target->RemoveSystemEventListener(NS_LITERAL_STRING("fullscreenchange"),
mFullScreenListener,
/* useCapture */ true);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "RemoveSystemEventListener failed");
}

mFullScreenListener = nullptr;
Expand Down Expand Up @@ -530,18 +532,18 @@ ScreenOrientation::Notify(const hal::ScreenConfiguration& aConfiguration)
mAngle = aConfiguration.angle();
mType = InternalOrientationToType(orientation);

nsresult rv;
DebugOnly<nsresult> rv;
if (mScreen && mType != previousOrientation) {
// Use of mozorientationchange is deprecated.
rv = mScreen->DispatchTrustedEvent(NS_LITERAL_STRING("mozorientationchange"));
NS_WARN_IF(NS_FAILED(rv));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "DispatchTrustedEvent failed");
}

if (doc->Hidden() && !mVisibleListener) {
mVisibleListener = new VisibleEventListener();
rv = doc->AddSystemEventListener(NS_LITERAL_STRING("visibilitychange"),
mVisibleListener, /* useCapture = */ true);
NS_WARN_IF(NS_FAILED(rv));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "AddSystemEventListener failed");
return;
}

Expand All @@ -557,7 +559,7 @@ ScreenOrientation::Notify(const hal::ScreenConfiguration& aConfiguration)
nsCOMPtr<nsIRunnable> runnable = NewRunnableMethod(this,
&ScreenOrientation::DispatchChangeEvent);
rv = NS_DispatchToMainThread(runnable);
NS_WARN_IF(NS_FAILED(rv));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "NS_DispatchToMainThread failed");
}
}

Expand All @@ -567,16 +569,16 @@ ScreenOrientation::UpdateActiveOrientationLock(ScreenOrientationInternal aOrient
if (aOrientation == eScreenOrientation_None) {
hal::UnlockScreenOrientation();
} else {
bool rv = hal::LockScreenOrientation(aOrientation);
NS_WARN_IF(!rv);
DebugOnly<bool> ok = hal::LockScreenOrientation(aOrientation);
NS_WARNING_ASSERTION(ok, "hal::LockScreenOrientation failed");
}
}

void
ScreenOrientation::DispatchChangeEvent()
{
nsresult rv = DispatchTrustedEvent(NS_LITERAL_STRING("change"));
NS_WARN_IF(NS_FAILED(rv));
DebugOnly<nsresult> rv = DispatchTrustedEvent(NS_LITERAL_STRING("change"));
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "DispatchTrustedEvent failed");
}

JSObject*
Expand Down
Loading

0 comments on commit d375269

Please sign in to comment.