Segmentation fault passing struct pointer to function

Question

I'm pretty newb in C and in order to learn about structs I'm building a program which provides a limited set of functionality for a struct ll_string which basically is a linkedlist of strings.

The set of functions I'm trying to implement includes an insert_ll_string() function which should concanate a passed in struct ll_string element to the end of another struct ll_string element but fails to do so because the moment the function is called in my test cases, the program crashes with a sig fault. This is at the STILL WORKS and SIG FAULT comments of the test_insert() function.

This is its header file:

file: ll_string.h

struct ll_string {
    char *string;
    struct ll_string *next;
};

struct ll_string *create_ll_string(char *, struct ll_string *);
void insert_ll_string(struct ll_string *, struct ll_string *);
void remove_item_from_ll_string(struct ll_string *, struct ll_string *);
void free_ll_string(struct ll_string *);
void print_ll_string(struct ll_string *);

and this is it's corresponding .c file missing a few definitions for functions declared in ll_string.h, but I guess my problem probably only revolves around the functions create_ll_string() and insert_ll_string() anyways.

file: ll_string.c

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

#include "ll_string.h"

/* create_ll_string: allocates memory for a new struct ll_string and 
 * initializes it with given arguments returns a pointer to new struct */
struct ll_string *create_ll_string(char *string, struct ll_string *next) {
    struct ll_string *new_ll_string;

    if (!string) {
        printf("string can\'t be NULL\n");
        return NULL;
    }

    if (*string == '\0') {
        printf("string needs to be at least 1 char long\n");
        return NULL;
    }

    if (!(new_ll_string = (struct ll_string *) malloc(sizeof(struct ll_string)))) {
        printf("couldn\'t allocate mem for new ll_string\n");
        exit(EXIT_FAILURE);
    }

    new_ll_string->string = strdup(string);
    new_ll_string->next = next;

    return new_ll_string;
}

/* insert_ll_string: concanates item to the end of dest */
void insert_ll_string(struct ll_string *dest, struct ll_string *item) {
    struct ll_string *cur;

    if (!dest) {
        printf("dest and item can\'t be NULL\n");
        return;
    }
    if (!item) {
        printf("item can\'t be NULL\n");
        return;
    }

    cur = dest;
    while (!cur->next) {
        cur = cur->next;
    }

    cur->next = item;

    return ;
}

/* remove_item_from_ll_string: removes item from list src */
void remove_item_from_ll_string(struct ll_string *src, struct ll_string *item) {
    return ;
}

/* printf_ll_string: prints each string in ll_string */
void print_ll_string(struct ll_string *ll_string) {
    if (!ll_string) {
        printf("ll_string is NULL\n");
        return ;
    }

    do {
        printf("%s\n", ll_string->string);
    } while (!(ll_string = ll_string->next));
}

/* free_ll_string: frees all memory pointed to by ll_string */
void free_ll_string(struct ll_string *ll_string) {
    struct ll_string *next;

    if (!ll_string) {
        return ;
    }

    while ((next = ll_string->next)) {
        free(ll_string->string);
        free(ll_string);
        ll_string = next;
    }
}

and here are my tests. Everything works fine until insert_ll_struct() is evoked by the test_insert() function. (test_create() works as expected) Tests are done using the MinUnit framework.

file: tests_ll_string.c

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

#include "minunit.h"

#include "ll_string.h"

#define MAX_ERROR_MSG_LENGTH 1000

int tests_run = 0;

static char *test_create(void) {
    struct ll_string *test_ll;
    struct ll_string *test_null_ll;
    char *empty_string = strdup("");
    char *null_string = NULL;
    char *correct_string = strdup("this should work");
    char *correct_string2 = strdup("this should also work");
    char *error_msg;

    if (!(error_msg = (char *) malloc(sizeof(char) * MAX_ERROR_MSG_LENGTH))) {
        printf("couldn\'t allocate mem for error msg");
        exit(EXIT_FAILURE);
    }

    // test_ll->string == correct_string
    // test_ll->next == NULL
    test_ll = create_ll_string(correct_string, NULL);
    sprintf(error_msg, "error, test_ll->string != \"%s\" is %s", correct_string, test_ll->string);
    mu_assert(
            error_msg,
            strcmp(test_ll->string, correct_string) == 0);

    // test_ll->next->string == correct_string
    // test_ll->string == correct_string2
    test_ll = create_ll_string(correct_string2, test_ll);
    sprintf(error_msg, "error, test_ll->string != \"%s\" is %s", correct_string2, test_ll->string);
    mu_assert(
            error_msg,
            strcmp(test_ll->string, correct_string2) == 0);

    sprintf(error_msg, "error, test_ll->next->string != \"%s\" is \"%s\"", correct_string, test_ll->next->string);
    mu_assert(
            error_msg,
            strcmp(test_ll->next->string, correct_string) == 0);

    test_null_ll = test_ll;
    test_null_ll = create_ll_string(empty_string, test_ll);
    // test_null_ll == NULL
    mu_assert(
            "error, test_null_ll != NULL",
            test_null_ll == NULL);

    test_null_ll = test_ll;
    test_null_ll = create_ll_string(null_string, test_ll);
    // test_null_ll == NULL
    mu_assert(
            "error, test_null_ll != NULL",
            test_null_ll == NULL);

    sprintf(error_msg, "error, test_ll->string != \"%s\" is \"%s\"", correct_string2, test_ll->string);
    mu_assert(
            error_msg,
            strcmp(test_ll->string, correct_string2) == 0);

    sprintf(error_msg, "error, test_ll->next->string != \"%s\" is \"%s\"", correct_string, test_ll->next->string);
    mu_assert(
            error_msg,
            strcmp(test_ll->next->string, correct_string) == 0);

    free_ll_string(test_ll);
    free(correct_string);
    free(correct_string2);
    free(empty_string);
    free(error_msg);

    return 0;
}

static char *test_insert(void) {
    struct ll_string *ll_test1;
    struct ll_string *ll_test2;
    struct ll_string *ll_test3;

    char *test_string1 = strdup("test_string1");
    char *test_string2 = strdup("test_string2");
    char *test_string3 = strdup("test_string3");
    char *error_msg;

    if (!(error_msg = (char *) malloc(sizeof(char) * MAX_ERROR_MSG_LENGTH))) {
        printf("couldn\'t allocate mem for error msg");
        exit(EXIT_FAILURE);
    }


    ll_test1 = create_ll_string(test_string1, NULL);
    ll_test2 = create_ll_string(test_string2, NULL);
    ll_test3 = create_ll_string(test_string3, NULL);

    // STILL WORKS

    insert_ll_string(ll_test1, ll_test2); // SEG FAULT
    insert_ll_string(ll_test1, ll_test3);

    sprintf(error_msg, "error, ll_test1->string != \"%s\" is \"%s\"", test_string1, ll_test1->string);
    mu_assert(
            error_msg,
            strcmp(ll_test1->string, test_string1) == 0);
    sprintf(error_msg, "error, ll_test1->next->string != \"%s\" is \"%s\"", test_string2, ll_test1->next->string);
    mu_assert(
            error_msg,
            strcmp(ll_test1->next->string, test_string2) == 0);
    sprintf(error_msg, "error, ll_test1->next->next->string != \"%s\" is \"%s\"", test_string1, ll_test1->next->next->string);
    mu_assert(
            error_msg,
            strcmp(ll_test1->next->next->string, test_string3) == 0);

    free_ll_string(ll_test1);
    free_ll_string(ll_test2);
    free_ll_string(ll_test3);
    free(test_string1);
    free(test_string2);
    free(test_string3);

    return 0;
}

static char *all_tests(void) {
    mu_run_test(test_create);
    mu_run_test(test_insert);
    return 0;
}

int main(int argc, char* argv[]) {
    char *result = all_tests();
    if (result != 0) {
        printf("%s\n", result);
    } else {
        printf("ALL TESTS PASSED\n");
    }
    printf("Tests run: %d\n", tests_run);

    return result != 0;
}

and this is the output of compilation and execution:

>> gcc -Wall -o test ll_string.c tests_ll_string.c
>> ./test
string needs to be at least 1 char long
string can't be NULL
[1]    6789 segmentation fault (core dumped)  ./test

What's causing this Sigmentation fault? I'm not accessing any memory besides local variables in the section the program crashes. I'm not dereferencing the pointers I'm passing to insert_ll_struct() at least not immediately after the function has been evoked.

Thanks in advance for the help

Use a debugger - it will tell you where the seg fault occurs — Ed Heal
– Ed Heal, Commented Jan 13, 2018 at 23:55
do you get the seg fault if you just run test_insert (i.e. disable test_create) — the4thamigo_uk
– the4thamigo_uk, Commented Jan 14, 2018 at 0:12
I was thinking about testing it with gdb but I still need to figure out how to use it yet but now is probably the right time to learn it. :) In the meantime I would still appreciate any answer to my question. — kachink
– kachink, Commented Jan 14, 2018 at 0:55

the4thamigo_uk · Accepted Answer · 2018-01-14 09:42:12Z

1

I think the answer is staring us in the face. In insert_ll_string() :

while (!cur->next) {

should be

while (cur->next) {

answered Jan 14, 2018 at 9:42

the4thamigo_uk

8756 silver badges9 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

the4thamigo_uk · Accepted Answer · 2018-01-14 00:24:01Z

0

I'd look at the logic in free_ll_string(). Are you sure you aren't freeing memory twice? In the code it looks like if frees up all strings in the chain. Therefore I think you will free test_ll multiple times in test_create. See if you still get the error when disabling test_create, and if not then you issue I think is probably resulting from undefined behaviour because you are free-ing things more than once...

It is good practice to set any freed pointer to NULL after freeing the memory that it is pointing to, then you will avoid this problem.

/* free_ll_string: frees all memory pointed to by ll_string */
void free_ll_string(struct ll_string *ll_string) {
    struct ll_string *next;

    if (!ll_string) {
        return ;
    }

    while ((next = ll_string->next)) {
        free(ll_string->string);
        free(ll_string);
        ll_string = next;
    }
}

answered Jan 14, 2018 at 0:24

the4thamigo_uk

8756 silver badges9 bronze badges

7 Comments

kachink Over a year ago

Thank you for your answer. The seg fault also occurs when I just run test_insert. I can't see me freeing memory twice. The free_ll_string function should free all strings in the linked list starting from the passed in element till the end of the list and should then free each struct as well.

the4thamigo_uk Over a year ago

Ah ok, maybe not the cause of this seg fault then, since test_null__ll is NULL, nor do you attempt to call free_ll_string on it. However, there is still a danger here I think: If you A = create ll_string("a", NULL) and then B = create_ll_string("b", A), then freeing B will free A, and then freeing B will free B again. This is because next is not duplicated in create_ll_string and/or the pointers are not set to NULL in free_ll_string. So, if a programmer is diligent in calling free_ll_string for each create_ll_string statement, they are going to hit this problem.

the4thamigo_uk Over a year ago

(In fact, setting the pointers to NULL in free_ll_string wouldnt help in this case, but it is good defensive style anyway)

the4thamigo_uk Over a year ago

Also calling A = create_ll_string("a", NULL) then free_ll_string(A) wont free anything, since while ((next = ll_string->next)) performs no iterations.

kachink Over a year ago

Ok. I rewrote parts of create_ll_string and free_ll_string. Here are the new versions: [Gist][gist.github.com/muchosteechos/04834bd351102cae53a2683dc8866a4f]

|

Rohit · Accepted Answer · 2018-01-18 14:49:00Z

0

while (!cur->next) { cur = cur->next; } This code will cause crash suppose cur->next return null than you are trying to acess null next that is invalid you should put null check for cur in while loop like thiswhile (null!=cur&& !cur->next)

answered Jan 18, 2018 at 14:49

Rohit

1529 bronze badges

Collectives™ on Stack Overflow

Segmentation fault passing struct pointer to function

3 Answers 3

Comments

7 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

7 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related