reznikmm / protobuf

The Google Protocol Buffers implementation in Ada
MIT License
32 stars 5 forks source link

Memory leak in `Append` for vectors #15

Closed mgrojo closed 2 years ago

mgrojo commented 2 years ago

There's a memory leak in the Append procedures generated for vectors. For example:

https://github.com/reznikmm/protobuf/blob/c99dbfdf845e5eb0395a4f21cc6a7d97a962ac44/source/compiler/generated/google-protobuf-descriptor.adb#L208-L211

The generated code should call Free when the array has to be reallocated.

   procedure Append (Self : in out T_Vector; V    : T) is
      Init_Length : constant Positive := Positive'Max (1, 256 / T'Size);
   begin
      if Self.Length = 0 then
         Self.Data :=  new T_Array (1 .. Init_Length);
     elsif Self.Length = Self.Data'Last then
         declare
            Aux_Data : T_Array_Access := Self.Data;
         begin
            Self.Data :=
               new T_Array'(Self.Data.all & T_Array'(1 .. Self.Length => <>));
            Free (Aux_Data);
         end;
      end if;
      Self.Length := Self.Length + 1;
      Self.Data (Self.Length) := V;
   end Append;

By the way, is there any specific reason for not using Ada.Containers.Vectors in this case, so you don't worry about low-level memory management?

reznikmm commented 2 years ago

Yeah, indeed. Thank you for the report.

The reason for not using generic package is to avoid unresolved forward dependencies. For example, you can't instantiate Ada.Containers.Vectors for a private type until you have a full type declaration. Custom private type for vector allows you have cross-dependencies, like

type A is record
  V1 : B_Vector;
end record;
type B is record
  V1 : A_Vector;
end record;
mgrojo commented 2 years ago

I've fixed this for me and will provide a pull request. I generated the code without block, because it was easier for me to construct the ada_pretty structure without it. Maybe you prefer other way.

reznikmm commented 2 years ago

thank you, Manuel!