fgets set to char * causes segmentation fault, Dynamic string

Question

I'm writing a program in c that looks through a standardly inputted text file with lines "string int". I had the program running fine with scanf originally, but now the name has to be a dynamic string(size known at runtime) and when I switched the name in grade_entry to a pointer it started segmentation faulting

typedef struct grade_entry {
       char *name;
       int grade;
} grade_entry;

struct grade_entry grade_list[100];

int main(){
    int grade;
    int done;
    int i=0;
    do{
            puts("not weee\n");
            done=(int)strlen(gets(grade_list[i].name));
            puts("weee\n");
    }while(1);
}

compiles and current output is:

not weee

segmentation fault(core dumped)

**Resolved: I allocated the pointer and then reallocated for size+1, adding the null to The end. It's unfortunate that I can't malloc immediately based off of the temp memory for scanf,fgets,gets. Oh well, thank you everyone for the help

Answer 1

You have to allocate memory to grade_entry.name, otherwise it is a char pointer pointing to an arbitrary memory location and writing to it is causing segmentation fault.

Answer 2

You have not allocated memory to the char* name field, but are storing values in this memory, which you don't own actually.

Along with that you are running an infinite loop and playing with the same array element(Not incrementing the 'i')

Following is the code that will work I guess.

typedef struct grade_entry {
   char *name;
   int grade;
} grade_entry;

int main(){
int grade;
int done;
char str[100];
grade_entry grade_list[10];
int i=0;    
do{
        puts("not weee\n");
        gets(str);
        grade_list[i].name=(char*)malloc(strlen(str));
        done=strlen(grade_list[i++].name);
        puts("weee\n");
  }while(i<10);
}

Answer 3

The fault lies here:

        done=(int)strlen(gets(grade_list[i].name));//Storing in a unallocated memory using an uninitialized pointer. BAD!

Besides in your code I nowhere see you incrementing/decrementing/changing the value of i, so basically you will be overwriting grade_list[0].name everytime the loop executes. That's bad too. Too much memory leak!!

And its an infinite loop with no break condition!

Try something like this:

char buffer[MAX_LENGTH_OF_A_STRING];
memset(buffer,0x00,MAX_LENGTH_OF_A_STRING);
do{
        puts("not weee\n");
        done=strlen(gets(buffer));//This is enough
        grade_list[i].name=malloc(strlen(buffer)+1);
        strncpy(grade_list[i].name,buffer,strlen(buffer)+1);
        memset(buffer,0x00,MAX_LENGTH_OF_A_STRING);
        i++;//You were not doing this
        puts("weee\n");
        //Put some condition to break this infinite loop, somewhere within the loop
}while(1);

Answer 4

You should not use gets() — ever. Pretend it does not exist. Pretend it will blow up your computer if you use it. Assume it will be used to crash your program — because it will.

The trouble is that you cannot tell gets() how much space it has to work with.

Use fgets() instead. If you're worried about reading lines that are long, then you should hope your system has the POSIX 2008 function getline() which handles reading lines of arbitrary length for you. You only have to deal with the array of character pointers, which is simpler.

In your program, the pointers will end up being copied to the pointers in your data structures.

It's all good clean fun; the crucial thing is to remember that if you didn't initialize the pointer, it points randomly, and you can't use a pointer safely until you know what it points to. So, initialize your pointers to point somewhere, or to NULL to indicate that they point nowhere. Never dereference a NULL pointer.