GP32 Init All New Pointers To Null


pea

developer
Joined
Oct 3, 2004
Messages
1,089
Age
45
Location
New Zealand
Website
www.projectitis.com
Hi all,

Trying to compile libmikmod I have already debugged one error where a pointer wasn't being initialised to NULL. Is there a way to get the compiler to automatically do this at compile time (set all new pointers to NULL?).

The reason I ask is because surely, libmikmod being the widely used library it is, I can't be the first to pick up this error, and the project can't be as established as it is with an error this fundemental - so the problem must be at compile time?
 
Is the pointer a global or a local variable? Globals can be solved by wiping all the ram at startup, but since local come from the stack, I don't think it's possible to initialise them automatically. Be nice if there was a compiler switch, but I doub't there will be.
 
Is this the sort of thing you want it to do for you? (from your libmikmod thread)
news->next = NULL;

Because news is a pointer it's hard for the compiler to know what to do at compile time.

Is it a struct you malloc()'d yourself? That is even further away from the compiler!

If it is a local struct you've declared there are warnings about uninitialised variables, but that is about all the help GCC will give you.

You could try running lint on the code, but I suspect it will throw up more warnings than are helpful.

Sorry to have to say you'll probably have to initialize them yourself (by doing a memset after the alloc?)
 
Well, the code already exists (libmikmod) and I thought if that bug that I came accross (as in gp32rich post above) existed after more than 10 years in development and use, then it must be the compiler that is different.

I don't know if the struct is malloced or not, I haven't looked that far, but I know from looking at most of the code, it uses compiler allocated (or whatever its called) structs in most cases. I just thought that maybe the following line:

int *ptr;

Could be compiled to something equivalent to:

Allocate memory for pointer
Set pointer to NULL

Using some or other compiler flag :(
 
That only supports mod though... libmikmod supports mod s3m it xm and bunches more already.

Huh does libmikmod support sound effects too? o_O

Edit: yeah it does. :)
 
yeah, if spiv can get it to support some extended module formats, it would rock, especially since its written for the GP32. Bring it on!

But in the meantime, I am going to try to get libmikmod to work!

EDIT: Is Spivs mod code uploaded anywhere yet? Can't seem to find it...
 
Globals are always supposed to go to zero (which is also NULL); if its a "automatic" (within function) variable you define the default yourself. But even for globals, its good form to always specify a default value... and I've run into a bunch of compilers that forget to set defaluts, or even forget to apply your own coded default (sigh).. damn compilers ;)

jeff
 
skeezix posted on Mar 9 2005 at 02:27 PM said:
Globals are always supposed to go to zero (which is also NULL);

don't think so - afaik, if you don't initialise a variable in C or C++, the initial value is undefined (i.e. could be whatever happened to be in memory before, bios temp variables, previous programs, etc etc)

always initialise, it takes only seconds and it's much less of a headache
 
Last edited by a moderator:
Global variables that are not initialised belong to the bss section, and are initialised to zero at program startup. Local variables are not initialised at all, and may contain any value.

However, if you have several files, all of which contain globals, the order of initialisation of the globals is undefined.
 
All I know is that The code hung, and I traced it to a linked list walk that went something like this:
Code:
nextemptyitem = listitem->next;
while( nextemptyitem ){ nextemptyitem = nextemptyitem ->next; }
doSomethingWith( nextemptyitem );
And I discovered that when listitems are created, the 'next' pointer is not set to null, so I set it to null, and all was fine.

There is only one place in the code where these listitems are created, and only one place where the 'next' pointer is accessed, and between those two places, the next pointer was never set to null.

Hence two conclusions:
1) It is obviously a bug
2) For it to have been working in the past, the pointer must have been set to null some other way (i.e. by the compiler)

EDIT: I also looked at other version of the code, and the code has never changed (this part anyway).
 
how is the "listitem" created, is it with malloc/calloc/new/etc ?

Maybe it uses a library function that automatically zeroes memory, such as calloc, and the library you are using is not correctly doing this.
 
sloader.c, SL_LoadSample

snippet:
Code:
SAMPLOAD *news
...
if(!(news=(SAMPLOAD*)_mm_malloc(sizeof(SAMPLOAD)))) return NULL;

// This is the linked list walk. 'cruise' is also a SAMPLOAD.
if(cruise) {
	while(cruise->next) cruise=cruise->next;
	cruise->next = news;
} else
	*samplist = news;
...
news->infmt = s->flags & SF_FORMATMASK;
news->outfmt = news->infmt;
news->reader = reader;
news->sample = s;
news->length = s->length;
news->loopstart = s->loopstart;
news->loopend = s->loopend;
news->next = NULL; // Pea: Added to fix bug

The definition of _mm_malloc is:
Code:
// Pea 20050226 Changed for Mr Mirko SDK (GP32)
d = malloc(size);
if (!d){
	_mm_errno = MMERR_OUT_OF_MEMORY;
	if(_mm_errorhandler) _mm_errorhandler();
}
memset(d,size,0);
return d;

So, yes, with my definition of malloc, it should be working ok (clearing to null) but it doesn't seem to.
 
I always thought that with memset, the size was last, which would explain your bugs.

EDIT: Hmm, in the version of GCC I'm using (3.4.3), the prototype for memset is:

_PTR _EXFUN(memset,(_PTR, int, size_t));

Which tells me the last parameter is indeed the number of bytes to clear, which is zero in your example, so I'd say this is where the problem is.
 
\/\/0000t! Thank you Squidge!
The modules now load no problem, and only crash when they start to play. This is actually good news, because it means I can get on to debugging the playing code now (custom for GP32) - but it also means that the loading code (custom for GP32) is ok.

Interesting note. The reson that I got the order of the args wrong is because I commented out what was there, and then re-wrote it for the Mirko SDK - however, the code that was there used a custom memset function that had the arge the wrong way!

Code:
/* Same as DCALLOC, but sets error variable _mm_error when fails */
void* _mm_calloc(size_t nitems,size_t size)
{
	void *d;
   
	/*
	if(!(d=gp_mem_func.DMALLOC(size*nitems))) {
  _mm_errno = MMERR_OUT_OF_MEMORY;
  if(_mm_errorhandler) _mm_errorhandler();
	}
	gp_str_func.memset(d,size*nitems,0);
	return d;
	*/
	
	// Pea 20050226 Changed for Mr Mirko SDK (GP32)
	d=malloc(size*nitems);
	if(!d) {
  _mm_errno = MMERR_OUT_OF_MEMORY;
  if(_mm_errorhandler) _mm_errorhandler();
	}
	memset(d,0,size*nitems);
	return d;
}

I take it all back - blaming the compiler. I was just a dumbarse :)

EDIT: Interestingly, it seems to work and play of in geepee32, except it doesn't have sound output so I can't tell. On my BLU+, it crashes as soon as it starts playing (no sound is produced yet except for the clicking during load, which I will attempt to fix too :) )
 
Make sure you try to work in those file interpreters from the updated mikmod. ;)

And thank you so much for doing this. I really appreciate it. :)

Edit: Also the player code. Look at the updated version of that too. :p
 
No problem, its going to benefit the whole community, so is worth it:
1) Devers can use more music formats in games
2) Gamers can enjoy better game music
 
Back
Top