Search code examples
clistlinked-listsegmentation-faultcoredump

Segmentation fault (core dumped) error when making a linked list


I'm learing C for school, and one of the assignments was to create a database. Now I'm trying to add some of the inputs I give it to the list, but I keep getting a segmentation error. What am I doing wrong?

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

struct carinfo_t {
    char* carbrand;
    char* carmodel;
    int caryear;
    float carvalue;
    struct carinfo_t * next;
};

struct carinfo_t * carbase;

struct carinfo_t * tempcar;

struct carinfo_t * tempcar2;

struct carinfo_t * tempprint;

void freeCarinfo(struct carinfo_t * carinfo){
    free(carinfo->carbrand);
    free(carinfo->carmodel);
    free(carinfo);    
}

struct carinfo_t * createCarinfo(char *carbrand, char *carmodel, int caryear, float carvalue){
    struct carinfo_t * newcar;
    newcar = (struct carinfo_t *)malloc(sizeof (struct carinfo_t));
    newcar->carbrand=(char *)malloc(sizeof(char)*(strlen(carbrand) + 1));
    strcpy(newcar->carbrand, carbrand);

    newcar->carmodel=(char *)malloc(sizeof(char)*(strlen(carmodel) + 1));
    strcpy(newcar->carmodel, carmodel);

    newcar->caryear=caryear;

    newcar->carvalue=carvalue;

    newcar->next= NULL;

    return newcar;
}

struct carinfo_t * addCarinfo(struct carinfo_t *carbase, struct carinfo_t *newcar){
    if(carbase=NULL){
        carbase = newcar;
        return carbase;
    }

    else{
        tempcar2->next=carbase;
        carbase=tempcar2;
        return carbase;

    }

}

void printCarbase(struct carinfo_t *carbase){

    struct carinfo_t *tempprint = carbase;

    if (carbase == NULL){
        printf("The database contains no cars\n");
    }

    else{
        while (tempprint != NULL){
            printf("Car:\n");
            printf("- brand: %s\n", carbase->carbrand);
            printf("- model: %s\n", carbase->carmodel);
            printf("- year: %d\n", carbase->caryear);
            printf("- value: %7.2f\n", carbase->carvalue);
            tempprint = tempprint->next;
        }
    }
}


void main(void){
    struct carinfo_t * carbase;
    carbase = NULL;

    struct carinfo_t * tempcar;
    tempcar = createCarinfo("Opel", "Manta", 1965, 20000);

    struct carinfo_t * tempcar2 = createCarinfo("Ford", "Focus", 1999, 350.25);
    addCarinfo(carbase, tempcar);
}

Also, if you see any way to improve my code please tell me, I'm very new to programming, and I'd like to be able to do this properly.

edit: Thanks to everyone who responded, I figured out how to use GDB. Now that the original issue is fixed, I got the same error but this time it's "tempcar2" that seems to be the issue:

Program received signal SIGSEGV, Segmentation fault.
0x000000000040072a in addCarinfo (carbase=0x602010, newcar=0x602080)
    at database.c:56
56              tempcar2 = tempcar2->next;
(gdb) bt
#0  0x000000000040072a in addCarinfo (carbase=0x602010, newcar=0x602080)
    at database.c:56
#1  0x0000000000400869 in main () at database.c:98

Solution

  • Some problems in your code:

    • some variable are not initialized (tempcar2 for instance, which is global)
    • some local and global variable have the same name
    • some comparison are done with = instead of == operator
    • in your print function, you always print carbase instead of tempprint
    • malloc return is casted, it should not

    Some problems could have been detected by the compiler: turn on compiler warnings (-Wall for most compilers)

    When your code is compiled with warning on, your problems are revealed:

    .code.tio.c: In function ‘addCarinfo’:
    .code.tio.c:46:8: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
         if(carbase=NULL){
            ^~~~~~~
    .code.tio.c: At top level:
    .code.tio.c:81:6: warning: return type of ‘main’ is not ‘int’ [-Wmain]
     void main(void){
          ^~~~
    .code.tio.c: In function ‘main’:
    .code.tio.c:88:24: warning: unused variable ‘tempcar2’ [-Wunused-variable]
        struct carinfo_t * tempcar2 = createCarinfo("Ford", "Focus", 1999, 350.25);
                            ^~~~~~~~
    

    A corrected version could be:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    struct carinfo_t
    {
        char* carbrand;
        char* carmodel;
        int caryear;
        float carvalue;
        struct carinfo_t * next;
    };
    
    void freeCarinfo(struct carinfo_t * carinfo)
    {
        free(carinfo->carbrand);
        free(carinfo->carmodel);
        free(carinfo);    
    }
    
    struct carinfo_t * createCarinfo(char *carbrand, char *carmodel, int caryear, float carvalue)
    {
        struct carinfo_t * newcar;
        /* malloc cast is not recommender */
        newcar = malloc(sizeof (struct carinfo_t));
    
        /* strdup can be used here */
        newcar->carbrand = strdup(carbrand);    
        newcar->carmodel = strdup(carmodel);
    
        newcar->caryear=caryear;
        newcar->carvalue=carvalue;
    
        newcar->next= NULL;
    
        return newcar;
    }
    
    struct carinfo_t * addCarinfo(struct carinfo_t *carbase, struct carinfo_t *newcar)
    {
        if(carbase==NULL)
        {
            carbase = newcar;
            return carbase;
        }
        else
        {
            /* find for the last element */
            struct carinfo_t * tempcar2 = carbase;
            while(tempcar2->next)
            {
                tempcar2 = tempcar2->next;
            }
            /* add the new car to the list */
            tempcar2->next=newcar;
    
            return carbase;
        }
    }
    
    void printCarbase(struct carinfo_t *carbase)
    {
        struct carinfo_t *tempprint = carbase;
    
        if (carbase == NULL)
        {
            printf("The database contains no cars\n");
        }
        else
        {
            while (tempprint != NULL)
            {
                printf("Car:\n");
                printf("- brand: %s\n", tempprint->carbrand);
                printf("- model: %s\n", tempprint->carmodel);
                printf("- year: %d\n", tempprint->caryear);
                printf("- value: %7.2f\n", tempprint->carvalue);
                tempprint = tempprint->next;
            }
        }
    }
    
    int main(void)
    {
       struct carinfo_t * carbase;
       carbase = NULL;
    
       struct carinfo_t * tempcar;
       tempcar = createCarinfo("Opel", "Manta", 1965, 20000);
    
       struct carinfo_t * tempcar2 = createCarinfo("Ford", "Focus", 1999, 350.25);
       carbase = addCarinfo(carbase, tempcar);
       carbase = addCarinfo(carbase, tempcar2);
       printCarbase(carbase);
    
       return 0;       
    }
    

    The result with this code is:

    Car: 
    - brand: Opel
    - model: Manta
    - year: 1965
    - value: 20000.00
    Car: 
    - brand: Ford
    - model: Focus
    - year: 1999
    - value:  350.25