Closed Bug 1343292 Opened 7 years ago Closed 7 years ago

change GenericPrinter::put and ...::vprintf to return bool

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

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.
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 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+
(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.
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/051ef53a3a81
change return types in GenericPrinter; r=nbp
https://hg.mozilla.org/mozilla-central/rev/051ef53a3a81
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.