When copying a GregorianDateTime object B to A using GregorianDateTime.copyFrom() we don't deallocate the timeZone of A before allocating a new timeZone buffer and copying the timeZone of B. Hence, we are leaking memory. Instead, we should deallocate the timeZone buffer of A then allocate a new timeZone buffer for A and copy the timeZone from B into it.
Created attachment 77060 [details] Patch
Comment on attachment 77060 [details] Patch Can't we use an OwnArrayPtr here? That would be much cleaner.
Comment on attachment 77060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77060&action=review > JavaScriptCore/wtf/DateMath.h:204 > + delete [] timeZone; should we also set it to '0', in case if (rhs.timeZone) fails?
Created attachment 77077 [details] Patch Updated patch based on Eric Seidel's suggestion.
(In reply to comment #3) > (From update of attachment 77060 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77060&action=review > > > JavaScriptCore/wtf/DateMath.h:204 > > + delete [] timeZone; > > should we also set it to '0', in case if (rhs.timeZone) fails? Notice, we do this by line 209 in <http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/DateMath.h?rev=72884#L209>.
Comment on attachment 77077 [details] Patch OK... so now I wonder why we wouldn't just use Vector<char> here.
Comment on attachment 77077 [details] Patch This is strictly better than what we have, so OK.
Attachment 77077 [details] did not build on win: Build output: http://queues.webkit.org/results/7292081
Comment on attachment 77077 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77077&action=review Should find out why the Windows build broke. > JavaScriptCore/wtf/DateMath.cpp:1132 > - tm.timeZone = NULL; > + tm.timeZone.clear(); Side note: The most modern idiom for this is to assign to nullptr. We are going to deprecate clear(). > JavaScriptCore/wtf/DateMath.h:202 > + timeZone.set(new char[inZoneSize]); The most modern idiom for this is: timeZone = adoptArrayPtr(new char[inZoneSize]); We are going to deprecate set soon.
(In reply to comment #9) > (From update of attachment 77077 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77077&action=review > > Should find out why the Windows build broke. Will look into and fix before landing. > > > JavaScriptCore/wtf/DateMath.cpp:1132 > > - tm.timeZone = NULL; > > + tm.timeZone.clear(); > > Side note: The most modern idiom for this is to assign to nullptr. We are going to deprecate clear(). Will change. > > > JavaScriptCore/wtf/DateMath.h:202 > > + timeZone.set(new char[inZoneSize]); > > The most modern idiom for this is: > > timeZone = adoptArrayPtr(new char[inZoneSize]); Will change. > > We are going to deprecate set soon.
Committed r74424: <http://trac.webkit.org/changeset/74424>