Closed Bug 1070689 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: arai, Assigned: gokcehankara)

References

Details

Attachments

(1 file)

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, jliu@netscape.com
>        JS_ASSERT(0);
>        return rv;
>    }

It should be "return -1".
Blocks: 1019838
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.
Flags: needinfo?(continuation)
(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.
Flags: needinfo?(continuation)
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.
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.
> 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.
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.
Attachment #8495905 - Flags: review?(nicolas.b.pierron)
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.
Attachment #8495905 - Flags: review?(nicolas.b.pierron) → review+
Thank you Nicolas..
https://hg.mozilla.org/mozilla-central/rev/7f08e93cc2a5
Assignee: nobody → gokcehankara
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
(In reply to gokcehankara from comment #9)
> Thank you Nicolas..

Do you know what you want to continue working on?
(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.
(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.
You need to log in before you can comment on or make changes to this bug.