Nested structures

[chigley]

Hi,

I wonder if anyone can help me. I have the following structures defined

typedef struct _FirewallRule FirewallRule;
struct _FirewallRule{
		char	dnsValue[1024];
		char	ipAddress[14]; //xxx.xxx.xxx.xxx - 15 chars
		int		protocol;
		char	user [255];
		char	group [255];
		int		action;
		int		ruleStatus;
};

typedef struct _Program Program;
struct _Program{
	int 	id;
	char  	path[1024];
	char  	name[255];
	FirewallRule * firewallRules;
	int 	firewallRulesCount;
};

I have an array of Program structs, successfully populated, and the function for adding a program is listed below

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;
	newProgram.firewallRulesCount = 0;
	
	void *temp = (Program*)realloc(programs, (lNumPrograms + 1) * (sizeof(Program)));
	if ( temp != NULL ) {
		programs = temp;
		programs[lNumPrograms] = newProgram;
		lNumPrograms++;
	}
	else
	{
		//Raise an error to the user, their system may be running low on memory
	}
	return;
}

and this works happily. The code is still a little wet, as I need to implement the else case.

The function I am having problems with is the one that adds a FirewallRule struct to the array of FirewallRule structs nested in the the program struct.

The code is listed below

void addRuleToProgram(const int id, FirewallRule * rule)
{
	int rulesCount=0;
	if (rule==NULL){return;}
	
	FirewallRule newRule = *rule;
	printf("newRule dnsValue = %s \n",newRule.dnsValue);
	printf("program[%d].name = %s \n",id,programs[id].name);
	
	//Allocate some more memory to the rules list on the program record
	rulesCount = programs[id].firewallRulesCount;
	void *temp = (Program*)realloc(programs[id].firewallRules, (rulesCount + 1) * (sizeof(Program)));
	if (temp!=NULL)
	{
		programs[id].firewallRules = temp;
		//programs[id].firewallRules[rulesCount]=newRule;
		rulesCount++;
		programs[id].firewallRulesCount = rulesCount;
	}
}

This appears to allocate the memory ok, but when I comment the line back in

programs[id].firewallRules[rulesCount]=newRule;

it blows up. I think from what I am reading in the debugger, it is suggesting that newRule points to nothing. However my printfs at the top of the function are working so it cannot be pointing to nothing.

So I am doing something wrong and cannot see what. I hope someone more experienced can guide me on best practice and point out the error of my ways.

Thanks

Charlie Benger-Stevenson
Hart Hill IT Ltd

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

 

[Salem]

> char ipAddress[14]; //xxx.xxx.xxx.xxx - 15 chars
Isn't it obvious?
15 chars + \0 just don't fit in 14

Why are you casting realloc?
If you're getting a warning along the lines of "cannot convert void* into type*", then you're using a C++ compiler to compile C code.

If you're getting a warning along the lines of "cannot convert int into type*", then you're using a C compiler, but you forgot to #include <stdlib.h>
This can result in some seriously broken code on some machines.



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

 

[chigley]

None of the above. Although the 15 chars + \0 is a good spot.

Need to declare the nested array as

FirewallRules ** firewallRules;

This I think is that the parent structure cannot have dynamically allocated memory structures in it, rather you need a pointer to the array.

Seems to work. Any comments from C gurus would be welcome to double check my maths.

Charlie Benger-Stevenson
Hart Hill IT Ltd

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

 

[chigley]

The memory is getting corrupted somehow.

I have some debug code

printf("rule.ipAddress FOR PROGRAM %s = %s \n",programs[0].name,programs[0].firewallRules[0]->ipAddress);

When I execute this right after the rules have been added, it is fine.

When I execute this later in the program (like when I actually want to disaply them) the memory is corrupt.

That is to say, the parent structure is in tact, but the memory pointed to by

FirewallRules ** firewallRules;

has become corrupt.

This is looking to all intents and purposes as corruption of the heap.

So is this the correct way to store an array of structs within a parent struct?

Really really stumped, would appreciate some help.

Charlie Benger-Stevenson
Hart Hill IT Ltd

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

 

[Salem]

If you're using say windbg (or the debugger in visual studio), you can set a "data access" breakpoint on an address which becomes corrupt.

So at the moment the data becomes corrupt, the debugger will show you where it starts to go wrong.


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

 

[chigley]

I havent managed to find any way of connecting a debugger to gcc (MingW) yet. Can anyone suggest a link?

Charlie Benger-Stevenson
Hart Hill IT Ltd

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

 

[JoeCsI]

Hi there,

why you keep reallocating everytime you add a new firewall rule ?
Just to keep the list together as an array?

I mean:
Each program has its own rules list (you already have a rule count) which grows dinamically. For that, you need to make some changes on your structures:
1) Add field "struct _FirewallRule * prox;" to the struct _FirewallRule;
2) Treat the "FirewallRule * firewallRules;" as the list beginning pointer;
3) Each time you add a new rule, just add to the linked list. No need to realloc.

However ... if you prefer to keep your code as it is ...

Check this line:

void *temp = (Program*)realloc(programs[id].firewallRules, (rulesCount + 1) * (sizeof(Program)));

You're actually adding a new PROGRAM element to the firewallRule array - misstyping issue. Instead, you should ask memory reallocation for "(rulesCount+1) * sizeof( FirewallRule )". Cause you want to add a firewall item, not a program one.

Thank's GOD for the copy&Paste creator insight ...

Cheers,
Joe

 

[JoeCsI]

Sorry ...

I forgot to add modify else ...

You're code line should be exchanged for this one:

void * temp = ( FirewallRule * ) realloc( programs[ id ].firewallRules, ( rulesCount + 1 ) * sizeof( FirewallRule ) );

HIH (Hope it helped!)

Cheers,
Joe