more help

[surfxtc79]

Im having a problem when I try to free memory that has been previosly allocated. Ive checked to ensure the same address is being passed all the way through. I've attached the project and workspace for anyones whose interested in helping me. The error is occuring in the free_mystruct() function in ex6func.c.

Thanks,
Chris

[Prelude]

>I've attached the project and workspace for anyones whose interested in helping me.
It doesn't work that way. You post (in code tags) a program that exhibits your problem that we can play with. I don't download files just so that you can give me your 500 line program to debug. If it's too long to post, make it smaller.

[surfxtc79]

These are the functions that allocate and free memory:

//	Question 8
void cop_setvals(struct mystruct *ps,char *name,const char *id,int val){
	ps->name=name;
	strcpy(ps->id,id);
	ps->val=val;
	return;
}

//	Question 9
struct mystruct *alloc_mystruct(char *name,const char *id,int val){
	struct mystruct *pMyStruct=(struct mystruct *)malloc(1*sizeof(struct mystruct));
	
	pMyStruct->name=(char*)malloc(sizeof(name)+1);

	strcpy(pMyStruct->name, name);
	strcpy(pMyStruct->id, id);
	pMyStruct->val=val;

	return pMyStruct;
}

void free_mystruct(struct mystruct *ps){
	free(ps->name);
	free(ps);
	return;
}

//	Question 10
struct mystruct *copy_mystruct(struct mystruct *ps){
	struct mystruct *pMyStruct=alloc_mystruct(ps->name,ps->id,ps->val);

	return pMyStruct;
}

[surfxtc79]

this abridged version of the calls from main

	.
	.
	.
	.
	int sel=0;
	char buf1[80],buf2[80],buf3[80];
	char *ret1;
	int val1,val2;
	struct mystruct demo,*pdemo;
	FILE *myout,*myin;
	char inbuf[80],outbuf[80];
	.
	.
	.
	.
	.
			
			case 8:
			{
				fprintf(myout,"Enter name of structure: ");
				fgets(buf1,80,myin);
				if (buf1[strlen(buf1)-1]=='\n') buf1[strlen(buf1)-1]='\0';
				fprintf(myout,"Enter structure id (10 chars max): ");
				fgets(buf2,80,myin);
				if (buf2[strlen(buf2)-1]=='\n') buf2[strlen(buf2)-1]='\0';
				fprintf(myout,"Enter structure value (an integer): ");
				fgets(buf3,80,myin);
				if (buf3[strlen(buf3)-1]=='\n') buf3[strlen(buf3)-1]='\0';
				val1=atoi(buf3);
				cop_setvals(&demo,strdup(buf1),buf2,val1);
				fprintf(myout,"\nName: %s\nId: %s\nValue: %i\n",
							demo.name,demo.id,demo.val);
				break;
			}
			case 9:
			{
				fprintf(myout,"Enter name of structure: ");
				fgets(buf1,80,myin);
				if (buf1[strlen(buf1)-1]=='\n') buf1[strlen(buf1)-1]='\0';
				fprintf(myout,"Enter structure id (10 chars max): ");
				fgets(buf2,80,myin);
				if (buf2[strlen(buf2)-1]=='\n') buf2[strlen(buf2)-1]='\0';
				fprintf(myout,"Enter structure value (an integer): ");
				fgets(buf3,80,myin);
				if (buf3[strlen(buf3)-1]=='\n') buf3[strlen(buf3)-1]='\0';
				val1=atoi(buf3);
				pdemo=alloc_mystruct(buf1,buf2,val1);
				if (pdemo!=NULL)
				{
					fprintf(myout,"\nName: %s\nId: %s\nValue: %i\n",
							pdemo->name,pdemo->id,pdemo->val);
					free_mystruct(pdemo);
				}
				else fprintf(myout,"\nStructure allocation failed...\n");
				break;
			}
			case 10:
			{
				pdemo=copy_mystruct(&demo);
				if (pdemo!=NULL)
				{
					fprintf(myout,"\nName: %s\nId: %s\nValue: %i\n",
							pdemo->name,pdemo->id,pdemo->val);
					free_mystruct(pdemo);
				}
				else fprintf(myout,"\nStructure allocation failed...\n");
				break;
			}
	.
	.
	.
	.
	.
	.
	.

[Prelude]

I have a better idea, how about you write a bare bones program that still exhibits the problem instead of posting code that I have to patch together to even compile. You'll get help more quickly if you give us as little code to look at as possible that we can copy/paste and compile as posted. I usually just ignore posts that require me to do more than paste the code into a text editor and compile right away.

[surfxtc79]

my structure definition:

struct mystruct
{
	char *name;
	char id[10];
	int val;
};

[surfxtc79]

im not looking for anyone to debug it...i just want someone to look over the syntax, etc, and make sure im allocating and releasing memory in the correct way

[Prelude]

>Im having a problem when I try to free memory that has been previosly allocated.
>make sure im allocating and releasing memory in the correct way
Then you should have been more specific.

cop_setvals:

>ps->name=name;
This isn't a good idea if you previously allocated memory to ps->name as suggested by alloc_mystruct. By reassigning where the pointer points to, you lose all references to the memory and this creates a leak. Better to use strcpy or strncpy like you do with id.

Of course, this function is redundant as alloc_mystruct sets the values.

alloc_mystruct:

>struct mystruct *pMyStruct=(struct mystruct *)malloc(1*sizeof(struct mystruct));
First, you don't need the cast and including it can hide an error. Second, why are you multiplying the size of struct mystruct by 1? The sizeof operator returns a size_t value, which is sufficient. Also, to save yourself some maintenance later, you would do better to use a typedef for the type and change the line to something like this:

RECORD *pRecord = malloc(sizeof (*pRecord));

Whenever you call malloc, calloc, or realloc, always check the return value. On failure they all return a null pointer and it's an error to dereference a null pointer.

>pMyStruct->name=(char*)malloc(sizeof(name)+1);
This is a problem waiting to happen, the previous comments apply, but calling sizeof on a pointer returns the size of a pointer, not the array that it points to. On my system you would be allocating an array of size 5. A 4 byte pointer plus 1. This line would be better as (assuming previous changes are made):

pRecord->name = malloc(strlen(name) + 1);

>strcpy(pMyStruct->id, id);
It may be prudent to test id and make sure that pMyStruct->id has enough space to hold it.

copy_mystruct:
The temporary pointer isn't needed, you can just do this:

return alloc_mystruct(ps->name, ps->id, ps->val);

This should give you a start toward improving your code. Good luck.