1070689 - Uninitialized variable 'rv' returned in dosprintf in jsprf.cpp.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

dosprintf returns uninitialized variable 'rv'.

http://dxr.mozilla.org/mozilla-central/source/js/src/jsprf.cpp#621
>    if (!BuildArgArray(fmt, ap, nas)) {
>        // the fmt contains error Numbered Argument format, [e-mail]
>        JS_ASSERT(0);
>        return rv;
>    }

It should be "return -1".

Comment 1

[gokcehankara]

Looks like we missed that on the final version of the patch in the previous bug, sorry for that. We had talked about changing `dosprintf` and other functions here to return a `bool` instead of `int`. This could be done in this bug if it's appropriate. I'm already familiar with the code here so I can do it if noone else wants.

Comment 2

[Andrew McCreight [:mccr8]]

(In reply to gokcehankara from comment #1)
> I'm already familiar with the code here so I
> can do it if noone else wants.

Sounds good to me, thanks.

Comment 3

[Chris Peterson [:cpeterson]]

This bug is the last -Wuninitialized warning that clang and gcc report in mozilla-central. After it's fixed, I hope to enable -Wuninitialized warnings as errors in bug 1001975.

Comment 4

[gokcehankara]

I'm having a busy week at the moment, so I was hoping to take a look at this at the end of the week. If someone wants to take over it's fine by me. Alternatively, it might be easier to fix this bug simply with Tooru's suggestion (by changing `return rv;` to `return -1;`) and leave the refactoring to a different bug so that it won't block bug 1001975.

Comment 5

[Nicholas Nethercote [inactive]]

> This bug is the last -Wuninitialized warning that clang and gcc report in
> mozilla-central.

Oh, nice. I thought that making -Wuninitialized an error was a hopeless goal, but it looks like that's no longer the case.

Comment 6

[gokcehankara]

Attachment: 1070689.patch - Change return type of dosprintf functions from int to bool (in jsprf.cpp)

I'm attaching a patch for changing the return type of functions in here (jsprf.cpp) from int (0 for success, -1 for failure) to bool (true for success, false for failure) except for public functions so they should work as before. This should fix this bug.

I haven't set anyone for review as I wasn't sure who to pick. Also I still don't have repo access so I'm gonna require some help with that either.

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8495905 [details] [diff] [review]
1070689.patch - Change return type of dosprintf functions from int to bool (in jsprf.cpp)

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

Nice work.

Comment 8

[Nicolas B. Pierron [:nbp]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c27cabb2bf74
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f08e93cc2a5

Comment 9

[gokcehankara]

Thank you Nicolas..

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f08e93cc2a5

Comment 11

[Nicolas B. Pierron [:nbp]]

(In reply to gokcehankara from comment #9)
> Thank you Nicolas..

Do you know what you want to continue working on?

Comment 12

[gokcehankara]

(In reply to Nicolas B. Pierron [:nbp] from comment #11)
> (In reply to gokcehankara from comment #9)
> > Thank you Nicolas..
> 
> Do you know what you want to continue working on?

I don't have anything in mind so I'm open to suggestions if you have any. Since my school has started I would prefer something I could work on with a small pace.

Comment 13

[Nicolas B. Pierron [:nbp]]

(In reply to gokcehankara from comment #12)
> (In reply to Nicolas B. Pierron [:nbp] from comment #11)
> > Do you know what you want to continue working on?
> 
> I don't have anything in mind so I'm open to suggestions if you have any.
> Since my school has started I would prefer something I could work on with a
> small pace.

I have a few bugs such as Bug 1062893, which are not urgent but are adding more capabilities of optimization to IonMonkey.