Thread: Function with struct pointer

So the problem is foo function where I should take person atributes do some changes with them and rewrite them to another file but fprintf function have some problem with p_person->name if anybody can help me, I would be very thankfull

its all written in main I just made it smaller...

Code:

struct person {
char* name;
} person;

FILE * pFile;
char * buffer;
char * line;


pFile = fopen ("data.dat","r");


fgets(buffer,100,pFile);
line = malloc (strlen(buffer)+1 * sizeof(char));
strcpy(line,buffer);


char* pointer=strchr(line,':');
*(pointer)='\0';


char* person_name = malloc (strlen(line)+1 * sizeof(char));
strcpy(person_name,line);


struct person* pMyPerson = malloc(sizeof(struct person));
pMyPerson->name = malloc (strlen(person_name)+1 * sizeof(char));
strcpy(pMyPerson->name,person_name);


fclose (pFile);
printf("%s",pMyPerson->name);

void foo(struct person* p_person){
FILE * output= fopen("datat.txt","w");
fprinf(output, "%s", (p_person)->name);
fclose (output);
}


foo(pMyPerson);


free(line);

You can't nest functions like this is C. Move foo outside whatever function the rest of the code is - and you'll have move your definition of struct person up above both of them as well.

Other minor stuff :

sizeof(char) is defined to be 1, no need to multiply by it in malloc
check the return values of various functions which set pointers (malloc, pointer, fopen, fgets, etc).
no point in copying from buffer to line to person_name only to copy it into pMyPerson->name. Skip the middle steps
pick a consistent variable naming scheme, preferably without using the abomination Hungarian notation

I did what u said, but still undefined referes in fprintf ... any ideas?
The whole code looks like this.
I am quite new at dinamic allocations so I made those midle steps to have less problems during freeing the memory I guess... because under unix its not so easy like under the windows
Anyways thanks for your help

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


struct person {
char* name;
} person;


void foo(struct person* p_person){
FILE * output= fopen("datat.txt","w");
fprinf(output, "%s", (p_person)->name);
fclose (output);
}


int main(){




FILE * pFile;
char * buffer;
char * line;




pFile = fopen ("data.dat","r");




fgets(buffer,100,pFile);
line = malloc (strlen(buffer)+1 * sizeof(char));
strcpy(line,buffer);




char* pointer=strchr(line,':');
*(pointer)='\0';




/*char* person_name = malloc (strlen(line)+1 * sizeof(char));
strcpy(person_name,line);
*/


struct person* pMyPerson = malloc(sizeof(struct person));
pMyPerson->name = malloc (strlen(line)+1 * sizeof(char));
strcpy(pMyPerson->name,line);




fclose (pFile);
printf("%s",pMyPerson->name);


//foo(pMyPerson);


free(line);
    return 0;
}

Typo?

Code:

fprinf(output, "%s", (p_person)->name);

Referring to message 3...

line 36 and 37 can be deleted.

line 42 and 43 can operate directly on your input buffer...

Code:

pointer = strchr(buffer,':')
if (poniter)
  *pointer=0;

... which will save you the trouble of repeatedly copying your strings (which is slow).

Lines 54 and 55 would then read...

Code:

pMyPerson->name = malloc (strlen(buffer)+1 * sizeof(char));
strcpy(pMyPerson->name,buffer);

Line 67 is no longer needed if you get rid of the intermediate line variable (Lines 36 and 37).

Also you need to free your person struct or the string within when done with them.

As matters of general practice, you should always check your file pointers to be sure the file is actually opened before reading, writing or closing the file. In many cases a file that doesn't open should terminate the program.

Aside from the fprintf typo, buffer needs to point to something before being used.

Just a thought. You should always cast the pointer returned by malloc to the right type.


struct person* pMyPerson = (struct person*)malloc(sizeof(struct person));


does pMyPerson->name prints the right value?

Originally Posted by mnd22

Just a thought. You should always cast the pointer returned by malloc to the right type.

No you should not typecast the return value of malloc() in C ... C++ (stupidly) forces you to... but in C malloc returns a void* which can be assigned to any pointer without a typecast. In fact, typecasting it incorrectly will simply introduce more errors into the program.

If your compiler is complaining that you aren't typecasting malloc... you are compiling your code as C++, not C.

(Aside: I wonder how many coders will one day discover this and realize that in all their experience they've never actually written a real C program)