libnds: glTexImage2D use of DynamicArray

[Al_Bro]

Hi,

I did a quick search and couldn't find this - but I'm new to these boards so apologies if this has already been covered. I'm using libnds source version 1.3.3.

glTexImage2D uses a dynamic array to track the association between texture name allocation and physical memory addresses. However, it appears to assume that the contents of the dynamic array are set to 0 after resizing - which is not guaranteed and can cause the routine to copy texture data to random memory (as occurred in my application).

DynamicArraySet (in dyanmicArray.h):

Code: Select all

void DynamicArraySet(DynamicArray *v, int index, void* item)
{
	if(index >= v->cur_size)
	{
		v->cur_size *= 2;
		v->data = (void**)realloc(v->data, sizeof(void*) * v->cur_size);
	}
	
	v->data[index] = item;
}

This uses realloc which does not guarantee the state of memory after reallocation. Adding a reference to <string.h> in dynamicArray.h and changing the routine to:

Code: Select all

void DynamicArraySet(DynamicArray *v, int index, void* item)
{
	if(index >= v->cur_size)
	{
		v->data = (void**)realloc(v->data, sizeof(void*) * v->cur_size * 2);
		memset(v->data + v->cur_size, 0, sizeof(void*) * v->cur_size);
		v->cur_size *= 2;
	}
	
	v->data[index] = item;
}

Of course, this has a slight performance overhead but there is currently no feedback to the caller of DynamicArraySet that a re-size operation has occurred and that data within the new area may be undefined.

If there's a better way of reporting things such as this then please let me know - I'm pretty new to the platform and very new to this forum.

[dovoto]

Been away so appologize for the late reply. Your suggestion will be included in next release and thanks for pointing it out.

[ain faren lasen]

I also want to suggest something ...

Shouldn't

Code: Select all

void DynamicArrayDelete(DynamicArray* v)
{
	if(v->data) free(v->data);
}

rather be

Code: Select all

void DynamicArrayDelete(DynamicArray* v)
{
	if(v->data) free(v->data);
	v->cur_size = 0;
}

?

Regards,
ain

[Al_Bro]

That's probably a good idea - but I don't think that call is actually called from anywhere at the moment. To be honest, there are other potential problems with the DynamicArray implementation. For example, code such as:

Code: Select all

DynamicArray *NewArray = new DynamicArray;
DynamicArrayInit(NewArray, 16);
DynamicArraySet(NewArray, 33, NULL);

will fail as DynamicArraySet assumes that a write to any index not currently within the bounds of the array will fall between cur_size and (cur_size * 2 - 1). Allocating more than 32 texture names and filling them in reverse order would cause this problem. Also, simply calling DynamicArraySet or DynamicArrayGet with a negative index will also corrupt memory or return invalid data.

That's not to knock the code - it's probably the right size for what it does without adding too much overhead and it's pretty easy to crash the DS in other ways.