Program using realloc on global variable

[chigley]

Hi,

I have a struct which is defined in a header file (firewallrules.h) thus

typedef struct _FirewallRule FirewallRule;
struct _FirewallRule{
		char * dnsValue;
		char * ipAddress;
		int protocol;
		char * user;
		char * group;
		int action;
		int ruleStatus;
};

typedef struct _Program Program;
struct _Program{
	int 	id;
	char * 	path;
	char * 	name;
	FirewallRule * firewallRules;
};

It is declared as global thus

extern Program * programs;
extern int lNumPrograms;

and defined in a C file thus

Program * programs = NULL;
int lNumPrograms=0;

I have a file called programsearch.c that includes firewallrules.h

It has a routine called addProgram which adds a Program item to the Programs array.

void addProgram(char * name, char * path)
{
	Program newProgram;
	printf("Adding program %d %s \n", lNumPrograms ,name);
	newProgram.id=lNumPrograms;
	strcpy(newProgram.name,name);
	strcpy(newProgram.path,path);
	newProgram.firewallRules = NULL;
	programs = (Program*)realloc(programs, (lNumPrograms + 1) * (sizeof(Program)));
    programs[lNumPrograms] = newProgram;
    lNumPrograms++;
	
	return;
}

It used to work beatifully when the _Program struct was declared in the programsearch.h file but since I have moved it and declared it using the extern keyword I have been getting some erratic behaviour.

It does work, but only for the first 19 times it is called, then it blows up.

Can someone give me some idea as to what the problem might be, and how to go about tracking it down.

Charlie Benger-Stevenson
Hart Hill IT Ltd

http://www.harthill-it.co.uk

 

[Salem]

Try

Program newProgram = { 0 };

Well, not really.
But it should demonstrate the reason by blowing up on the first call, and not blowing up at some random point in the future, because of a seemingly unrelated change.

The answer is of course that you're not allocating any space for your name and path members.

> programs = (Program*)realloc(programs, (lNumPrograms + 1) * (sizeof(Program)));
This is unsafe. If realloc returns NULL, then you've leaked your old memory.
Use

void *temp = realloc(programs, (lNumPrograms + 1) * (sizeof(Program)));
if ( temp != NULL ) {
  programs = temp;
} else {
  // do something with programs, which is still valid
  // as if realloc was never called.
  // eg. save and exit, or popup 'no more programs'
}

--
If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.

 

[chigley]

Great answer. Things to try, so I am not feeling as stranded. Thanks for your suggestions.

Charlie Benger-Stevenson
Hart Hill IT Ltd

http://www.harthill-it.co.uk