What's the cause of my segmentation fault?

Question

I'm trying to write a program that reads in entries from a file into a dynamically allocated array of structures using input redirection. My program compiles fine but I'm getting a segmentation fault and I'm having trouble finding the cause.

Here's my Program:

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

typedef struct StudentData
{
char* name;
char* major;
double gpa;
} Student;

int main()
{
Student* data = (Student*)malloc(sizeof(Student)*5);

int i;
for(i = 0; i < 5; i++)
{
 // allocate memory for name and read input
 data[i].name = malloc(50);
 *(data+i)->name == scanf("%s", (char*)&data[i].name);

 // allocate memory for major and read input
 data[i].major = malloc(30);
 *(data+i)->major == scanf("%s", (char*)&data[i].major);

 // read input for gpa
 (data+i)->gpa == scanf("%lf", &data[i].gpa); 

 //print array
 printf("%s\n%s\n%f\n", data[i].name, data[i].major, data[i].gpa);
 }
}  

Any clues? If it seems obvious it's because I'm relatively new to C!

Answer 1

This line:

*(data+i)->name == scanf("%s", (char*)&data[i].name);

Ignoring the weird and void == for a moment, &data[i].name is wrong since you're taking an address of a pointer. data[i].name would be sufficient here, since the name field is already an address scanf can write into.

And the cast to (char*) is what probably shuts the compiler up about it - did you enter it for this purpose :-) ? Because &data[i].name has the type char**, which scanf wouldn't accept, unless you forcefully casted it to char*.

As a general advice, try to avoid scanf - it leads to very unsafe code (as you've just seen!) Instead, use fgets to read a line (from the standard input too) and then break this line into its constituents. This may initially take a bit more code to implement, but leads to much safer and more predictable code.

Answer 2

*(data+i)->name == scanf("%s", (char*)&data[i].name);

What are you comparing the return value of scanf for? Just remove the first part. Also, data[i].name is already a pointer, so you shouldn't take the address once again. It should just be:

scanf("%s", data[i].name);  // no & because name is already a pointer

And similarly:

scanf("%s", data[i].major);
scanf("%lf", &data[i].gpa);  // & here because gpa is just a double

Answer 3

&data[i].name and &data[i].major are of type char **, so you cannot safely cast it to char *.

Losing the ampersand will correct your error.

There are also other logical errors with the use of scanf(), but that's probably overwhelming - it'd be nice if you revisited this code once you entered a name of more than 50 characters.

Answer 4

There is some unnecessary code being used with scanf, like *(data+i)->name ==. That doesn't do anything useful (and is probably causing the segfault). If it weren't causing access errors, it would compare the return value of scanf with the pointer and then ignore the result of the comparison. (A decent compiler would have warned about this.)

After getting rid of the excess code, it will be technically okay, except there is nothing to prevent buffer overrun. That's done either by controlling the input data, or adding limits to the lengths of the strings, like with scanf("%50s", data[i].name);