Liste mit TRects - Speicherleck?

[igel457]

Hallo, ich bin es mal wieder.

Ich habe eine Liste, in die man Rechtecke des Typs TRect einfügen kann (siehe unten).

Meine Frage ist nun, mache ich dies so richtig oder laufe ich womöglich Gefahr ein Speicherleck zu erzeugen?

Delphi-Quellcode:

procedure TRectList.Add(ARect: TRect);
var ar:PRect;
begin
  new(ar);
  ar^.Left := ARect.Left;
  ar^.Top := ARect.Top;
  ar^.Right := ARect.Right;
  ar^.Bottom := ARect.Bottom;
  inherited Add(ar);
end;

procedure TRectList.Clear;
var i:integer;
begin
  for I := 0 to Count - 1 do
  begin
    FreeMem(inherited Items[i]);
    Delete(i);
  end;
end;

function TRectList.GetItem(AIndex:integer):TRect;
begin
  result := PRect(inherited Items[AIndex])^;
end;

procedure TRectList.SetItem(AIndex:integer;AItem:TRect);
begin
  inherited Items[AIndex] := @AItem;
end;

Danke für eure Hilfe,
igel457

[jakobwenzel]

Setz einfach mal Application.ReportMemLeaksOnShutdown auf True, und du weißt, ob du memleaks hast. :wink:

[igel457]

Hm...

ReportMemLeaksOnShutdown finde ich leider nicht. Muss ich da noch irgendwas einbinden?

[DGL-luke]

Sollte das nicht ohne Application funktionieren?

[Hansa]

Du verwendest oft inherited. Z.B. hier :

Delphi-Quellcode:

  inherited Items[AIndex] := @AItem;

Was ist z.B. das ? :shock: Von wo wird denn da was geerbt ?

[igel457]

Die Liste stammt von TList ab... habe ich vergessen zu sagen...

[SirThornberry]

dein Clear müsste irgendwann ein "Index out of Bounds" bringen weil du die Schleife vorwärts laufen lässt aber die Elemente der reihe nach löschst. Somit werden es immer weniger Items pro Durschlauf und die letzten Schleifendurchläufe klappen nicht mehr.
Deine Methode SetItems macht überhaupt keinen Sinn, denn AItem liegt auf dem Stack und ist nach verlassen der Methode nicht mehr gültig. Somit hängst du einen Pointer in deine Liste welcher auf eine Variable auf dem Stack zeigt welche früh oder Später überschrieben wird.

und in der Add-Methode kannst du anstelle von

Delphi-Quellcode:

ar^.Left := ARect.Left; 
ar^.Top := ARect.Top; 
ar^.Right := ARect.Right; 
ar^.Bottom := ARect.Bottom;

auch gleich schreiben

Delphi-Quellcode:

ar^ := ARect;

gleiches kannst du auch in der Set-Methode machen, schließlich ist der Speicher schon durch Add reserviert wenn dem Index ein neuer Rect-Wert zugewiesen wird.
Genau da könnte auch dein Speicherleck sein. In der Add-Methode vorderst du mit "new" speicher an und hängst den Pointer auf diesen Speicher in die Liste. In der SetMethode überschreibst du diesen Pointer einfach ohne den alten Speicher frei zu geben. Aber wie gesagt macht es auch keinen Sinn den Pointer dort zu überschreiben. Sinvoller ist es das zu überschreiben wo der Pointer hin zeigt.

[igel457]

Danke für die Antworten, jetzt sieht das ganze so aus:

Delphi-Quellcode:

{TRectList}

procedure TRectList.Add(ARect: TRect);
var ar:PRect;
begin
  new(ar);
  ar^ := ARect;
  inherited Add(ar);
end;

procedure TRectList.Clear;
var i:integer;
begin
  while Count > 0 do
  begin
    FreeMem(inherited Items[i]);
    Delete(0);
  end;
end;

function TRectList.GetItem(AIndex:integer):TRect;
begin
  result := PRect(inherited Items[AIndex])^;
end;

procedure TRectList.SetItem(AIndex:integer;AItem:TRect);
var ar:PRect;
begin
  FreeMem(inherited Items[AIndex]);
  new(ar);
  ar^ := AItem;
  inherited Items[AIndex] := ar;
end;

Ist das so aktzeptabler?

Schonmal ein dickes Dankeschön.

[SirThornberry]

schau dir mal dein neues Clear an, dein "i" ist dort fehl am Platz.
Und in der SetMethode ist das freigeben überflüssig. Du gibst dort speicher frei und forderst sofort danach neuen Speicher an. Sinnvoller wäre es den einmal reservierten Speicher weiter zu nutzen.

Delphi-Quellcode:

PRect(inherited Items[AIndex])^ := AItem;

und zu deinem Clear. Es geht auch so

Delphi-Quellcode:

var i: Integer;
begin
  for i := 0 to Count - 1 do
    Dispose(inherited Items[i]);
  inherited Clear();

Anstelle von Dispose kannst du natürlich auch weiter "FreeMem" nutzen.

[igel457]

Danke für die Hinweise, das mit dem "i" ist mir schon aufgefallen...

Delphi-Quellcode:

{TRectList}

procedure TRectList.Add(ARect: TRect);
var ar:PRect;
begin
  new(ar);
  ar^ := ARect;
  inherited Add(ar);
end;

procedure TRectList.Clear;
begin
  while Count > 0 do
  begin
    FreeMem(inherited Items[0]);
    Delete(0);
  end;
end;

function TRectList.GetItem(AIndex:integer):TRect;
begin
  result := PRect(inherited Items[AIndex])^;
end;

procedure TRectList.SetItem(AIndex:integer;AItem:TRect);
begin
  PRect(inherited Items[AIndex])^ := AItem;
end;

Ich arbeite so selten mit Pointern und Co, da bin ich mir nie so richtig sicher...

@SirThornberry
Was ist an deiner Clear Methode besser? Damit wird die Liste doch sogar zweimal durchlaufen...

Danke,
igel457

[SirThornberry]

bei meinem Clear wird die Schleife nur einmal durchlaufen da Clear der eigentlichen TList nicht item für item frei gibt sondern die größe der Liste mit einem mal ändert.

[Muetze1]

1. Wenn New(), dann bitte auch Dispose()
2. Dispose() sollte mit dem richtigen Typ aufgerufen werden.
3. Wozu gerade Clear überschreiben? Wenn ein Eintrag aus der Liste gelöscht wird, dann wird dieser nicht freigegeben!
4. Es gibt bei TList nicht umsonst ein Notify im protected Abschnitt (damit ist Punkt 3 auch erledigt):

Delphi-Quellcode:

Procedure TRectList.Notify(Ptr: Pointer; Action: TListNotification); 
Begin
  If ( Action = lnDelete ) Then
    Dispose(PRect(Ptr));

  Inherited;
End;

[SirThornberry]

biem Freigeben spielt es keine Rolle ob man Dispose den richtigen Typ übergibt. Denn beim Freigeben wird einfach geprüft ob die übergebene Speicheradresse vorher allociert wurde. Wenn dem der Fall ist werd auch genau dieser Speicherblock wieder frei gegeben.

[Muetze1]

Zitat:

Zitat von SirThornberry

biem Freigeben spielt es keine Rolle ob man Dispose den richtigen Typ übergibt. Denn beim Freigeben wird einfach geprüft ob die übergebene Speicheradresse vorher allociert wurde. Wenn dem der Fall ist werd auch genau dieser Speicherblock wieder frei gegeben.

Wenn aber andere Typen in dem Typ enthalten sind - z.B. Strings - welche noch abgeräumt bzw. "finalisiert" werden müssen, so geschieht dies nicht, wenn der Typ ein untypisierter Zeiger ist.