Incorrect return of NS_ERROR_* codes in functions returning PRBool

RESOLVED FIXED in mozilla8

Status

()

Core
Rewriting and Analysis
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

(Depends on: 1 bug)

Trunk
mozilla8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

Fix
13.46 KB, patch
mak
: review+
Ehsan
: review+
(dormant account)
: review+
Biesinger
: review+
Axel Hecht
: review+
khuey
: review+
dholbert
: review+
Josh Aas
: review+
bjacob
: review+
cpearce
: review-
briansmith
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 545596 [details] [diff] [review]
Fix

Problems discovered by static analysis and patched by hand.

This set of fixes touches stuff all over the place so I'm requesting reviews from many people. To be clear:

bjacob:
 content/canvas/src/WebGLContextValidate.cpp            |    2 +-

cpearce:
 content/media/ogg/nsOggCodecState.cpp                  |    2 +-

josh:
 dom/plugins/base/nsPluginStreamListenerPeer.cpp        |    3 +--

ehsan:
 editor/libeditor/base/nsSelectionState.cpp             |    2 +-

khuey:
 layout/forms/nsFileControlFrame.cpp                    |   10 +++++-----

dholbert:
 modules/libpr0n/src/VectorImage.cpp                    |    2 +-

biesi:
 netwerk/protocol/http/nsHttpChannelAuthProvider.cpp    |    9 ++++++---

pike:
 rdf/base/src/nsRDFContainerUtils.cpp                   |    9 ++++++---

bsmith:
 services/crypto/component/nsSyncJPAKE.cpp              |    2 +-

mak:
 browser/components/migration/src/nsProfileMigrator.cpp |    4 ++--
 toolkit/components/places/Helpers.h                    |   14 ++++++++++----
 toolkit/components/places/nsNavBookmarks.cpp           |    2 +-
 toolkit/components/places/nsNavHistory.cpp             |    4 ++--
 toolkit/components/places/nsNavHistoryResult.cpp       |   14 +++++++++-----

taras:
 toolkit/components/telemetry/Telemetry.cpp             |    2 +
Attachment #545596 - Flags: review?(tglek)
Attachment #545596 - Flags: review?(mak77)
Attachment #545596 - Flags: review?(l10n)
Attachment #545596 - Flags: review?(khuey)
Attachment #545596 - Flags: review?(joshmoz)
Attachment #545596 - Flags: review?(ehsan)
Attachment #545596 - Flags: review?(dholbert)
Attachment #545596 - Flags: review?(cbiesinger)
Attachment #545596 - Flags: review?(bjacob
Comment on attachment 545596 [details] [diff] [review]
Fix

r=me (on the VectorImage line)  Thanks for catching these!
Attachment #545596 - Flags: review?(dholbert) → review+

Comment 2

6 years ago
Comment on attachment 545596 [details] [diff] [review]
Fix

Review of attachment 545596 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on the RDF parts.
Attachment #545596 - Flags: review?(l10n) → review+
Comment on attachment 545596 [details] [diff] [review]
Fix

Nice work.
Attachment #545596 - Flags: review?(khuey) → review+

Comment 4

6 years ago
Comment on attachment 545596 [details] [diff] [review]
Fix

Review of attachment 545596 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #545596 - Flags: review?(joshmoz) → review+
Comment on attachment 545596 [details] [diff] [review]
Fix

Review of attachment 545596 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you!
Attachment #545596 - Flags: review?(mak77) → review+

Comment 6

6 years ago
Comment on attachment 545596 [details] [diff] [review]
Fix

oops
Attachment #545596 - Flags: review?(tglek) → review+
(Assignee)

Updated

6 years ago
Attachment #545596 - Flags: review?(bsmith)
Comment on attachment 545596 [details] [diff] [review]
Fix

Thanks for pointing out this error.

Unfortunately this condition is an unrecoverable error, and returning PR_FALSE from nsOggCodecState::PacketOutUntilGranulepos() doesn't enforce that. Just exclude the nsOggCodecState.cpp changes from your patch, and I'll make appropriate changes in another bug. Thanks!
Attachment #545596 - Flags: review?(chris) → review-
Depends on: 671438
Attachment #545596 - Flags: review?(bsmith) → review+
Comment on attachment 545596 [details] [diff] [review]
Fix

r=me on the editor hunk.
Attachment #545596 - Flags: review?(ehsan) → review+
Attachment #545596 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 9

6 years ago
Comment on attachment 545596 [details] [diff] [review]
Fix

Switching reviewers - bjacob -> joe on the WebGLContextValidate.cpp part.
Attachment #545596 - Flags: review?(bjacob) → review?(joe)
Comment on attachment 545596 [details] [diff] [review]
Fix

r+ for the WebGL part (back from vacation). Thanks for that.
Attachment #545596 - Flags: review?(joe) → review+
(Assignee)

Comment 11

6 years ago
Thanks for all the reviews.

http://hg.mozilla.org/integration/mozilla-inbound/rev/7a21ce9c4482
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/7a21ce9c4482
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.