views:

74

answers:

5

Hi all. I've got the task of making a car rental program, which uses linked lists to control what cars are available for rent, are rented, or are in repair. The 'car' is a structure, and so is the rented list, available list, and repair list.

Heres my current issue. If the user wants to make a new car available, we must add it to our list of all possible cars, and we must add it to the list of available cars.

I have no problem adding it to the list of cars, but when i need to add it to the list of available cars, i get a segmentation fault.

I will now provide the code:

typedef struct vehicles
{
    char idNum[20];
    int miles;
    int rDate;
    struct vehicles *nextCar;

}car;

typedef struct list
{
    car * aCar;
    struct list *nextCar;
} carList;

The list of all cars is:

car * carHead, * carCur;

The list of all available cars is:

carList * availHead, * availCur;

Both are initialized to NULL.

I then create a new car, and put in the data the user has given me (mileage and ID number)

carCur = (car *)malloc(sizeof(car));
//set ID, Mileage
for(k=0;k<=19;k++)
{
     carCur->idNum[k] = idNum[k];
}
carCur->miles = miles;
carCur->nextCar = NULL;

This works perfectly fine. I call the function which actually adds it to the list, all is well.

Then i create a new carList structure to be added to the available cars list.

availCur = (carList *)malloc(sizeof(carList));
//set ID, Mileage
for(k=0;k<=19;k++)
{
    availCur->aCar->idNum[k] = idNum[k];
    printf("assigned\n");
}
availCur->aCar->miles = miles;
availCur->nextCar = NULL;

After some testing using printf statements,(which werent all included here for brevity), i found the seg fault occurs in this statement.

    availCur->aCar->idNum[k] = idNum[k];

I'm hoping someone can tell me why this assignment results in a segfault. I've checked the idNum provided by the user is good, and it works for adding to the all-cars list, so I'm not sure what is wrong.

I appreciate the help!

+3  A: 

You are right about the point of seg fault.
You are allocating memory correctly

availCur = (carList *)malloc(sizeof(carList));

at this point availCur->aCar is a dangling pointer. Next you are dereferencing that pointer here

availCur->aCar->idNum[k]
              ^^

and this causes the crash.

To fix this you need to allocate memory for a car object and make availCur->aCar point to it before you start stuffing it.

codaddict
Thanks, i've selected your answer as chosen because it is the most concise, and answers the question very directly.
Blackbinary
+2  A: 

You haven't allocated any memory for aCar, which is a pointer to a car. When you try and reference a field inside aCar you're trying to access memory that simply doesn't exist, hence the segmentation fault.

Depending on what you want the available car list to hold there's a number of possible remedies. Suppose you want each entry in the available car list to correspond to an entry in the list of all cars, you can simply have your aCar field point at an existing car in the car list. On the other hand, if you want the available car list to hold its own set of cars, you need to first allocate memory for a car, then assign it to aCar and populate its fields.

Mark E
thanks for the help, and the explanation of my options. I am doing the latter, as that was the way I was directed to in the assignment.
Blackbinary
+1  A: 

A few things:

  1. Why do you have a linked list of cars, and a linked list of linked lists of cars (car and carList)?
  2. fprintf(stderr, "...") is better for debugging because the messages will not be delayed; printf("...") may be buffered, causing you to not have the most recent output when the program crashes.
  3. You did not assign availCar->aCar before dereferencing it. You need a availCar->aCar = (car *)malloc(sizeof(car)) before you can use availCar->aCar->....
Jonathan
thanks, i didn't know that difference. Will use fprintf now :)
Blackbinary
+1  A: 

This code:

for(k=0;k<=19;k++)
{
     carCur->idNum[k] = idNum[k];
}

is scary and very likely broken. If the user doesn't call with a string containing at least 20 valid characters, it runs a high risk of crashing. You want:

strcpy(carCur->idNum, idNum);
unwind
thankyou, its been a while for me, forgot about strcpy.
Blackbinary
A: 

The best would be to create a list class and put a constructor that will allocate memory for aCar member and a destructor to free this memory (of course you need to move to C++ to do that).

Benoit Thiery
I'm not sure thats a good suggestion. Your basically saying 'switch to OOP'. which doesn't help with the question.
Blackbinary