Есть ли тут утечка памяти?

[GrayEddy]

Перелопачиваю проект, доставшийся по наследству. В нем постепенно растет память, особенно виртуальная.
Решил посмотреть, где явно выделяется память. И вот наткнулся на такой кусок кода

Код: Выделить всё

procedure TMainForm.RemoveCommand(commIndex_: byte);
var i,j:integer;
begin
  for i:=0 to CommandCount-1 do
    if Commands[i].CommIndex=commIndex_ then break;
   if i<CommandCount then
     begin
       Сommands[i].Destroy;   // <---  1
       for j:=i to CommandCount-2 do
         Commands[j]:=commands[j+1]; // <---  2
       dec(CommandCount); 
       setlength(Commands,CommandCount);
    end;
end;

Здесь пояснения. Commands - динамический одномерныйц массив класса TCommand (который унаследован от TObject) длиной High(Byte).
В пункте 1 безусловно, надо заменить на Сommands[i].Free;
Алгоритм очень прост. По значению commIndex_ находим нужный нужный нам элемент массива и осовобождаем его.
Затем сдвигаем элементы массива справа налево правее освобожденного.
В результате этого в пункте 2 последний и предпоследний элементы указывают на один и тот же экземпляр класса. Вот этот момент и смущает.
Затем массив обрезается на 1 элемент, последний элемент уходит из зоны видимости.

[Light13]

Насчет FPC не скажу точно, но в delphi компилятор не гарантирует сохранность значения переменной цикла с параметром после выхода из цикла.

After the for statement terminates (provided this was not forced by a break or an exit procedure), the value of counter is undefined.

[GrayEddy]

Да, это Delphi проект.

[Light13]

Уменьшение длины массива не играет роли, там ведь только указатели на объекты хранятся
Попробуйте так сделать:

Код: Выделить всё

procedure TMainForm.RemoveCommand(commIndex_: byte);
  var
    i,j: Integer;
begin
  for i:=0 to CommandCount - 1 do
    if Commands[i].CommIndex = commIndex_ then
      begin
           Commands[i].Free;

           for j:=i to CommandCount - 2 do
             Commands[j]:=commands[j + 1];

           CommandCount:=CommandCount - 1;

           setlength(Commands, CommandCount);

           Break;
      end;
end;


Добавлено спустя 5 минут 17 секунд:
В своих проектах взял привычку добавлять модуль с кодом

Код: Выделить всё

interface

implementation

uses
  Windows;

initialization

finalization
  if AllocMemCount <> 0 then
    MessageBox(0, 'Ошибка освобождения памяти.' ,'Память не освобождена полностью.', MB_OK or MB_ICONERROR or MB_TASKMODAL);


затем прописываем этот модуль первым в файле проекта, поймали сообщение - смотрим код

[Bupyc]

Честно говоря, утечек памяти я здесь на первый взгляд не вижу, но есть пара моментов, которые мозолят глаза.

1. Не нравится мне использование цикловой переменной i после цикла. Компилятор никаких ворнингов случаем не выдает?
2. Если CommandCount = 0, то на строке setlength(Commands,CommandCount) FreePascal может вывалиться с Exception. Я обычно такое отслеживаю и если CommandCount = 0, то Commands := nil;

P.S. Вообще я бы этот кусок кода переписал. Времени уйдёт минут 10, и не нужно будет разбираться как он работает.

[GrayEddy]

1. Нет, варнингов нет. Мы не меняем значение i в самом цикле.
2. Спасибо, учту.

Думаю просто создать новый проект, там 2500 строк кода.

[Дож]

Если CommandCount = 0, то на строке setlength(Commands,CommandCount) FreePascal может вывалиться с Exception. Я обычно такое отслеживаю и если CommandCount = 0, то Commands := nil;

Это как такое можно получить?

Меня это беспокоит, потому что уже второй год использую массивы, и очень часто делаю SetLength(..., 0), но исключений не словил. Использую Win32 компилятор.

[Bupyc]

У меня было такое под линуксом, при компиляции через командную строку. Выскакивал exception что то типа Range Check Error. Возможно, лечится указанием соответствующей директивы компилятора. Я не разобрался, просто поправил нужный кусок кода.

[Дож]

«Range Check Error» скорее всего возник из-за попытки обратиться к какому-то элементу массива, после того как размер массива установлен в 0.
Commands[High(Commands)] вполне может к этому привести.

[Bupyc]

Нет, таких попыток не было. Exception был именно на строке SetLength.