Closed Bug 1376585 Opened 3 years ago Closed 3 years ago

[Static Analysis][Resource Leak] In function setCloneBuffer_impl

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1413003)

Attachments

(1 file)

The Static Analysis tool Coverity detected that |str| will leak in the following context:

        >>if (!str)
        >>    return false;
        >>size_t nbytes = JS_GetStringLength(args[0].toString());
        >>MOZ_ASSERT(nbytes % sizeof(uint64_t) == 0);
        >>auto buf = js::MakeUnique<JSStructuredCloneData>(0, 0, nbytes);
        >>if (!buf->Init(nbytes, nbytes))
        >>    return false;
Comment on attachment 8881536 [details]
Bug 1376585 - prevent memory leak in setCloneBuffer_impl.

https://reviewboard.mozilla.org/r/152708/#review157988

Good find. We could use UniqueChars so we don't have to free, but this seems fine for this testing function.
Attachment #8881536 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #2)
> Comment on attachment 8881536 [details]
> Bug 1376585 - prevent memory leak in setCloneBuffer_impl.
> 
> https://reviewboard.mozilla.org/r/152708/#review157988
> 
> Good find. We could use UniqueChars so we don't have to free, but this seems
> fine for this testing function.

Thanks for the review, i still have a question in case i will re-encounter issues of this sort, would you bee more comfortable using UniquePtr instead? I'm asking this since i've seen that's widely used in JS
(In reply to Jan de Mooij [:jandem] from comment #2)
> Comment on attachment 8881536 [details]
> Bug 1376585 - prevent memory leak in setCloneBuffer_impl.
> 
> https://reviewboard.mozilla.org/r/152708/#review157988
> 
> Good find. We could use UniqueChars so we don't have to free, but this seems
> fine for this testing function.

Opps, disregard my comment. I didn't read your previous comments.
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e77f10939e0f
prevent memory leak in setCloneBuffer_impl. r=jandem
https://hg.mozilla.org/mozilla-central/rev/e77f10939e0f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.