Closed
Bug 1343292
Opened 7 years ago
Closed 7 years ago
change GenericPrinter::put and ...::vprintf to return bool
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: tromey, Assigned: tromey)
References
Details
Attachments
(1 file)
As discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1334307#c3, GenericPrinter::put returns an int, but (1) the meaning of the result is inconsistent between subclasses, and (2) the callers sometimes use it incorrectly anyway. Instead, it can simply return bool indicating successs or failure.
Assignee | ||
Comment 1•7 years ago
|
||
I found a couple of spots in Disassemble1 that weren't checking the return value of put() (where other calls were checking the result), so I fixed those up as well.
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8842170 [details] Bug 1343292 - change return types in GenericPrinter; https://reviewboard.mozilla.org/r/116080/#review117834 ::: js/src/jsopcode.cpp:1379 (Diff revision 1) > for (size_t i = after - before; i < stack_column - 1; i++) > - sp->put(" "); > + if (!sp->put(" ")) > + return false; nit: add braces on the for-loop. ::: js/src/vm/Printer.h:39 (Diff revision 1) > - // the beginning of this new data. > - virtual int put(const char* s, size_t len) = 0; > + // return true on success, false on failure. the beginning of > + // this new data. nit: remove "the beginning of this new data".
Attachment #8842170 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #3) > nit: remove "the beginning of this new data". Oops. But thanks, I found a couple other comments to update as well.
Comment hidden (mozreview-request) |
Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/051ef53a3a81 change return types in GenericPrinter; r=nbp
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/051ef53a3a81
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•