Hi, I just implemented my first resizable container ( std::vector ) and it broke on the first attempt lol!

Heap corruption is caught when the assignment operator attempts to free() old data, and it works fine without free() , which of course is a leak.

Here's the use case : ( simplified from a recursive function ) 

	typedef struct Widget 
	{
		Vec3		 pos;
		float		 width;
		float		 height;
		List<Widget> children;
	};

	List<Widget> list ;

	Widget widget = {};

	widget.children.add (widget); 
	widget.children.add (widget); // Heap corruption when free() is called in assignment operator=()

 Definition :

template <typename T>
struct List
{
	T*	start	 = 0;
	u64 capacity = 0;
	u64 size	 = 0;

	      inline List  ( ) {};
	      inline List  ( u32 count );
	      inline List  ( const List<T>& other ) ;
		  inline ~List ( );

	inline void add     ( T element );
	inline void reserve ( u32 count );
	inline void clear   ();

	inline T& operator[] (u64 x) 
	{
		return start[x];
    }
  
	inline List<T>& operator=( List<T>& other );

};


template <typename T> 
List<T>::List   ( u32 count )
{
	u32 len		= sizeof(T) * count; 
	start		= (T*) malloc ( len );
	size		= 0;
	capacity	= len;
};

template <typename T>
List<T>::List  ( const List<T>& other ) 
{ 
	u64 len		= other.size * sizeof(T) ;

	if ( other.size ) 
	{
		start = (T*) malloc ( len );
		for ( int i=0; i < other.size; i++)
		{
			start[i] = other.start[i];
		}
	}
	size      = other.size;
	capacity  = len;
}

template <typename T> 
List<T>::~List   ( )
{
	if ( start )
		free ( start );
};

template <typename T> 
List<T>& List<T>::operator=( List<T>& other )
{ 
	u64 len	= other.size * sizeof(T) ;
	
	if ( other.size ) 
	{
		if ( start )
			free ( start );  // Corruption exception from here

		start = (T*) malloc ( len );
		for ( int i=0; i < other.size; i++)
		{
			start[i] = other[i];
		}
	}
	size      = other.size;
	capacity  = len;

	return *this;	
}

template <typename T> 
inline void List<T>::add   ( T element )
{ 
	if ( !start )
	{ 
		reserve ( 1 );
		start[size] = element;
		size++;

		return;
	}

	if ( size >= ( capacity / sizeof(T) ) )
	{
		reserve ( size + 1 );
	}
	
	start[size] = element;
	size++;
};

template <typename T> 
inline void List<T>::reserve   ( u32 count )
{ 	
	if ( count == 0 )
	{
		clear();
		return;
	}
	if ( count == size )
	{
		return;
	}

	T* old	 = start;
	u64 len  = sizeof(T) * count * 2;
	start    = (T*) malloc ( len );

	for ( int i=0; i < size; i++)
	{
		start[i] = old[i];
	}
	
	capacity	= len;

	if ( old )
		free ( old );
}

template <typename T> 
inline void List<T>::clear ()
{
	if ( start )
		free ( start );
	size		= 0;
	capacity	= 0;
};

For some reason it sometimes works without any exceptions being thrown..

16 minutes ago, rip-off said:

First, is there a reason you're not using std::vector?

Mainly out of curiosity and also the verbosity of STL, the syntax looks awful to me. Although i'm aware of the downsides of making a new one. 

1 hour ago, rip-off said:

Another problem is that add(...) calls T::operator=(...) on the element to be constructed, but that element will not be initialised. You should use placement new.

Thanks a ton! that was the issue. I didn't even know about placement new!

1 hour ago, rip-off said:

Something I find a bit confusing is that the "size" appears to count elements, whereas "capacity" appears to be in bytes. I think using elements for both would be simpler to reason about, and then you can convert to bytes when calling malloc(...). I also find the name "start" a little confusing, something like "elements" might be clearer.

Obviously it depends on what you've defined the member function to mean, but most containers have separate resize(...) and reserve(...) member functions, with the former potentially destructive (it could reduce the number of elements in the container) and the latter will only ensure the capacity is sufficient, it will never result in the container having less elements.

True i'll change them.

2 hours ago, rip-off said:

Another thing to think about is exception safety. For example, in your operator=(...), should there be an exception when the elements are being copied, your List becomes corrupted. Exception safety in general is very difficult in C++, to be fully exception safe and avoid a memory leak you'd also need to destroy the partially constructed elements during the copy, but memory corruption could open you up to security issues, particularly if your game interacts with untrusted data (e.g. multiplayer, or even just downloading custom maps / mods).

I may be wrong but for now i won't use any exceptions anywhere, to keep things cleaner and avoid their overhead.