View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005106 | JEDI Code Library | JclArrayLists | public | 2010-01-13 15:03 | 2010-01-24 06:02 |
| Reporter | dangi | Assigned To | Robert Rossmair | ||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | Version 2.2 | ||||
| Target Version | Fixed in Version | ||||
| Summary | 0005106: Memory corruption in Extract() and ExtractItems() | ||||
| Description | This also affects JclVectors.pas This is due to a misuse of JclBase.MoveArray() Count parameter. Resulting in a write outside of the array. This fix is also: - a complement to fix of bug 0004705 TJclIntfArrayList: Internal List contains References to Interfaces after clearing - rev 2698 fixed Extract() but not ExtractItem() http://jcl.svn.sourceforge.net/viewvc/jcl?view=rev&revision=2698 - another (cleaner?) way to fix bug 0005061 Possible memory leaks in MoveArray() for string and interface arrays - it is possible that changes of rev 3098 are no longer required (the FinalizeArrayBeforeMove() http://jcl.svn.sourceforge.net/viewvc/jcl?view=rev&revision=3098 Patch for the prototypes is attached, .pas files need to be updated accordingly. | ||||
| Tags | Containers, MoveArray | ||||
| Fixed in GIT commit | |||||
| Fixed in SVN revision | 3135 | ||||
| IDE version | RAD Studio 2007 | ||||
|
2010-01-13 15:03
|
jcl_containers_patch.txt (1,845 bytes)
Index: prototypes/containers/JclArrayLists.imp
===================================================================
--- prototypes/containers/JclArrayLists.imp (revision 2870)
+++ prototypes/containers/JclArrayLists.imp (working copy)
@@ -327,7 +327,7 @@
begin
FElementData[I] := DEFAULTVALUE;
if I < (FSize - 1) then
- MoveArray(FElementData, I + 1, I, FSize - I);
+ MoveArray(FElementData, I + 1, I, FSize - 1 - I);
Dec(FSize);
Result := True;
if FRemoveSingleElement then
@@ -358,8 +358,9 @@
if (Index >= 0) and (Index < FSize) then
begin
Result := FElementData[Index];
+ FElementData[Index] := DEFAULTVALUE;
if Index < (FSize - 1) then
- MoveArray(FElementData, Index + 1, Index, FSize - Index);
+ MoveArray(FElementData, Index + 1, Index, FSize - 1 - Index);
Dec(FSize);
AutoPack;
end
Index: prototypes/containers/JclVectors.imp
===================================================================
--- prototypes/containers/JclVectors.imp (revision 2870)
+++ prototypes/containers/JclVectors.imp (working copy)
@@ -326,7 +326,7 @@
begin
FItems[I] := DEFAULTVALUE;
if I < (FSize - 1) then
- MoveArray(FItems, I + 1, I, FSize - I);
+ MoveArray(FItems, I + 1, I, FSize - 1 - I);
Dec(FSize);
Result := True;
if FRemoveSingleElement then
@@ -357,8 +357,9 @@
if (Index >= 0) and (Index < FSize) then
begin
Result := FItems[Index];
+ FItems[Index] := DEFAULTVALUE;
if Index < (FSize - 1) then
- MoveArray(FItems, Index + 1, Index, FSize - Index);
+ MoveArray(FItems, Index + 1, Index, FSize - 1 - Index);
Dec(FSize);
AutoPack;
end
|
|
|
Thanks for the report. Also very considerate of you to supply a patch. Some Comments: The changes to MoveArray() in rev. 3098 are necessary; usability makes it mandatory for a function to release memory properly itself when possible and not leave the task to the caller. Within the Extract() methods, assigning DEFAULTVALUE (in the prototypes) to the array element impending to be overwritten is hence redundant; it would be redundant anyway for all types not needing finalization. These assignments have therefore been removed. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-01-13 15:03 | dangi | New Issue | |
| 2010-01-13 15:03 | dangi | File Added: jcl_containers_patch.txt | |
| 2010-01-13 15:03 | dangi | IDE version | => RAD Studio 2007 |
| 2010-01-13 18:47 | Robert Rossmair | Status | new => assigned |
| 2010-01-13 18:47 | Robert Rossmair | Assigned To | => Robert Rossmair |
| 2010-01-24 05:25 | Robert Rossmair | Tag Attached: MoveArray | |
| 2010-01-24 05:25 | Robert Rossmair | Tag Attached: Containers | |
| 2010-01-24 06:02 | Robert Rossmair | Fixed in revision | => 3135 |
| 2010-01-24 06:02 | Robert Rossmair | Note Added: 0017145 | |
| 2010-01-24 06:02 | Robert Rossmair | Status | assigned => resolved |
| 2010-01-24 06:02 | Robert Rossmair | Resolution | open => fixed |
