Issue 120108 - [From Symphony] There is a memory leak in function ScViewData::ReadUserDataSequence
[From Symphony] There is a memory leak in function ScViewData::ReadUserDataSe...
Status: CLOSED FIXED
Product: performance
Classification: Code
Component: code
AOO 3.4.0
All All
: P3 normal (vote)
: AOO 4.0
Assigned To: AOO issues mailing list
:
Depends on:
Blocks: 120975 121366
  Show dependency treegraph
 
Reported: 2012-06-28 05:52 UTC by ChaoHuang
Modified: 2013-02-16 09:10 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---


Attachments
for file "main/sc/source/ui/view/viewdata.cxx" (650 bytes, patch)
2012-06-28 06:05 UTC, ChaoHuang
hdu: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description ChaoHuang 2012-06-28 05:52:47 UTC
Steps to reproduce the defect:
1) Launch Aoo3.4
2) New a ods file
3) Close it

Defect : There is a memory leak in function ScViewData::ReadUserDataSequence
Comment 1 ChaoHuang 2012-06-28 06:04:07 UTC
In the first loop of function "ScViewData::ReadUserDataSequence", pTabData[nTab] will be reassigned to a new obj on heap. If the previous value is not NULL, there will be no place to release the previous obj. So it may be a memory leak. The solution is to check the pTabData[nTab] and free it if possible before reassignment.
Comment 2 ChaoHuang 2012-06-28 06:05:09 UTC
Created attachment 78507 [details]
for file "main/sc/source/ui/view/viewdata.cxx"
Comment 3 hdu@apache.org 2012-06-28 08:35:24 UTC
Comment on attachment 78507 [details]
for file "main/sc/source/ui/view/viewdata.cxx"

Thanks for finding the problem and the solution! Applied as revision 1354849

I apologize for slightly changing the fix because IMHO there is
- no need to check for NULL before delete, especially for stuff that is more often than not non-NULL
- the code comment did not make the code more clear and the details of the particular change belong into the revision history, but not into the code. If every change ever done in a file was commented like that it would become unreadable

That's only my opinion of course. If you as the author of the fix disagrees with that I'll reapply it in the original style.
Comment 4 hdu@apache.org 2012-06-28 08:36:54 UTC
updated status to resolved as the fix has been applied.
Comment 5 ChaoHuang 2012-06-28 08:44:46 UTC
(In reply to comment #3)
> Comment on attachment 78507 [details]
> for file "main/sc/source/ui/view/viewdata.cxx"
> 
> Thanks for finding the problem and the solution! Applied as revision 1354849
> 
> I apologize for slightly changing the fix because IMHO there is

> - no need to check for NULL before delete, especially for stuff that is more
> often than not non-NULL
   Yes, "delete NULL" is safe, anyway
 
> - the code comment did not make the code more clear and the details of the
> particular change belong into the revision history, but not into the code.
> If every change ever done in a file was commented like that it would become
> unreadable
   Totally agreed with your suggestion. The description for changing is needed in comments.

> 
> That's only my opinion of course. If you as the author of the fix disagrees
> with that I'll reapply it in the original style.

Thanks for your suggestion !
Comment 6 Yan Ji 2012-11-30 04:47:24 UTC
Since last SVT(r1400866) shows there is no memory leak, so close this defect as resolved.