Issue 120108

Summary: [From Symphony] There is a memory leak in function ScViewData::ReadUserDataSequence
Product: performance Reporter: ChaoHuang <chao.dev.h>
Component: codeAssignee: AOO issues mailing list <issues>
Status: CLOSED FIXED QA Contact:
Severity: Normal    
Priority: P3 CC: hdu, liushenf
Version: AOO 3.4.0   
Target Milestone: 4.0.0   
Hardware: All   
OS: All   
Issue Type: DEFECT Latest Confirmation in: ---
Developer Difficulty: ---
Issue Depends on:    
Issue Blocks: 120975, 121366    
Attachments:
Description Flags
for file "main/sc/source/ui/view/viewdata.cxx" hdu: review+

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.