Why am I getting a segmentation failure?

Question

Here is my code. I am doing some C practice for processing a file

I don't think the struct definition is the problem but I post it as well to give some context.

 typedef struct carType Car;
struct carType {
   int vehicleID;
   char make[20];
   char model[20];
   int year;
   int mileage;
   double cost;
   Car *next;
};

And the function that i think is causing the segmentation failure.

 void TextLoad(Car *headPointer)
   {
         char fileName[20];
         //prompt user for name of textfile to print to 
        scanf("%s", fileName);
        FILE *fpt;
        //estabish an IO connection
        fpt = fopen(fileName, "r");
        //current car to be printed
        Car *current;
        current = headPointer->next;
        while(fscanf(fpt,"%d %s %s cost:$%f mileage:%d, vehicleID:%d",&current->year,
    current->make, current->model,&current->cost,&current->mileage, &current->vehicleID) != EOF) 
        {
             current = current->next;
        }
        fclose(fpt);
    }  

The file that I tested this function had this content

 2014 Toyota Celica cost:$90000 mileage:5000, vehicleID:1
 2014 Toyota Rav4 cost:$4500 mileage:4000, vehicleID:2

What I have is basically a struct car and I want to use info from the file to initialize the fields of the struct car. Does anyone know where this segmentation failure is coming from? I checked on other threads fclose() causing segmentation fault, and Code fails with segmentation fault but I made sure to call fclose to close the IO connection and I didn't initialize another file pointer but that worked fine. I think the problem is the fscanf but I have the right format don't I?

Answer 1

You seem to have adapted a cycle that writes structures and use it to read structures.

The problem is that the area you're reading in might not be allocated. So your data is being read into the great black, and segfaults.

To work as it is, your function should receive a valid headPointer. You could probably do better by receiving a pointer to the headPointer itself, the value of headPointer being possibly NULL (i.e. if you've just started). If this is the case, you update the headPointer value, making it into a valid structure.

The same operation is repeated during the cycle.

For practicality, the operations connected to creating a new structure should be outsourced to a separate function.

The cycle should then:

*curPointerPtr = headPointerPointer;

while (!feof(fpt)) {
    // We need to allocate a new Car.
    *curPointer = newCar(); // Check it is not NULL (unless newCar throws an error)
    fscanf(into *curPointer)
    // prepare curPointer to accept next
    *curPointer = (*curPointer)->next;
}

The function to allocate a new car could also minimally initialize it (at least null all pointers and zero-terminate all strings):

Car *newCar(void) {
    Car *car;
    car = malloc(sizeof Car);
    if (NULL == car) {
        // Throw an error
    } else {
        // ALSO initialize the structure - good practice
        car->make[0] = 0x0;
        car->model[0] = 0x0;
        car->next = NULL;
    }
    return car;
}

Answer 2

It looks to me like you are not allocating space for the car struct. You create a pointer, but then you must create memory for it using malloc. I think you can get the size of your struct with sizeof(carType) as the argument for malloc. It's been a while since I used straight c like this, in C++ and C# you can just use new to call a constructor and the compiler does the memory management.

so it would look something like this:

Car *current = malloc(sizeof(carType));//you might want to try sizeof(car) if that doesn't work