segmentation fault (core dumped) using pointer

Question

When I build the code below, I get output

Mystr
segmentation fault (core dumped)

I guess strNULL and set function causeing error.. Am i guess wright???

I'm not sure what's wrong. please tell me what's wrong and how to fix it.
I would appreciate if advice my coding style :D

globalcall.c (global pointer declared)

void *call = 0;

string1.h (header file)

typedef struct tag_string{
    char *str;
    unsigned int len;

    int (*compare)(struct tag_string *target);
    struct tag_string *(*set)(struct tag_string *target);
} string;

string *new_String(const char *str);
void delete_string(string *str);

string.c

#include <stdlib.h>
#include "string1.h"

extern string *call;

int _string_compare(string *target);
string *_string_set(string *target);

string *new_string(const char *str){
    string *ptr;
    int i = 0;

    ptr = (string *)malloc(sizeof(string));
    if(ptr == 0){
        return 0;
    }

    ptr->str = 0;
    ptr->len = 0;

    ptr->compare = _string_compare;
    ptr->set = _string_set;

    if(str){
        for(ptr->len = 0;str[ptr->len];ptr->len++);

        ptr->str = (char *)malloc(ptr->len + 1);

        if(ptr->str == 0){
            free(ptr);
            return 0;
        }
        for(i = 0;i<ptr->len + 1;i++){
            ptr->str[i] = str[i];
        }
    } else {
        ptr->str = (char *)malloc(1);
        ptr->str = 0;
    }

    return ptr;
}

void delete_string(string *str){
    if(str->str != 0){
        free(str->str);
        str->str = 0;
    }
    free(str);

    return;
}

int _string_compare(string *target){
    int i = 0;
    if(call->len != target->len){
        return 0;
    } else {
        for(i=0;i<call->len;i++){
            if(call->str[i] != target->str[i]){
                return 0;
            }
        }
    }
    return 1;
}

string *_string_set(string *target){
    int i = 0;

    free(call->str);
    call->str = (char *)malloc(call->len+1);

    for(i = 0;i<call->len;i++){
        call->str[i] = target->str[i];
    }

    call->len = target->len;

    return call;
}

Main file

#include <stdio.h>
#include "string1.h"

extern void *call;

int main(void) {
    string *mystr = new_string("Mystr");
    string *strNULL = new_string(0);

    printf("%s\n",mystr->str);
    printf("%s\n",strNULL->str);

    if(strNULL->set(mystr)->compare(mystr)){
        printf("Compare : TRUE!");
    }

    printf("%s\n",mystr->str);
    printf("%s\n",strNULL->str);

    delete_string(mystr);
    delete_string(strNULL);

    return 0;
}

Answer 1

As far as I can see you never assign a value to the call variable.

And the variable is initialized to a null-pointer, hence any reference by call->anything triggers an Undefined Behavior.

Answer 2

It's a quite simple question, yet too tedious for the answers to seek the error in the code.

Here's my suggestion:
I assume you are running Linux Operating System
recompile your code with -g flag, and use gdb to debug

or you can set ulimit -c to large enough, and run the code again to get core file and see how it goes wrong.

Answer 3

The code is a bit of a mess. It is not clear why you are making it more complicated than it needs to be, but your segfault is here:

ptr->str = (char *)malloc(1);
ptr->str = 0;

I think what you mean is:

*(ptr->str) = 0;

Otherwise, you are creating a one-byte memory leak and setting the string to NULL, which will fail on:

printf("%s\n",strNULL->str);

Answer 4

The problem is occurring in function _string_set().

Problem no. 1:

You are dereferencing a pointer which is not initialized:

free(call->str);

Problem no. 2:

call is not initialized and you are trying to allocate memory to str:

call->str = (char *)malloc(call->len+1);

call->len is also not initialized.

Also, in main(), you are calling compare() for the call returned by _string_set() function but nowhere assigning the comparison function address to call->compare.

In short, your _string_set() function is full of undefined behaviors.

If I just make corrections in your _string_set() function, it will look like this:

string *_string_set(string *target){
    int i = 0;

    if (target == NULL)
            return NULL;

    if ((call != NULL) && (call->str != NULL)) {
            free(call->str);
            free(call);
    }

    call = malloc(sizeof(string));
    if (call == NULL)
            return NULL;

    call->len = target->len;
    call->str = malloc(call->len+1);
    if (call->str == NULL) {
            free(call);
            return NULL;
    }

    call->compare = target->compare;
    call->set = target->set;

    for(i = 0;i<call->len;i++){
            call->str[i] = target->str[i];
    }

    return call;
}

Note: These corrections in _string_set() is just to fix the segmentation fault in your program. There is a scope for improvement in your program.

I can see that in your program you are trying to set strNULL with values of mystr. For this, you need to pass the strNULL as well to _string_set() and in _string_set() make the appropriate changes.

With the current implementation, _string_set() will set the value of call member variable with mystr member variable values and your _string_set() returns call. The values of mystr will not reflect in strNULL.

---

Few suggestions for good coding practice:

1. Try not to use globals as much as possible.
2. Never cast malloc return. Check this.
3. Before accessing a pointer, make sure that it is pointing to a valid memory.
4. After calling malloc, always check whether it has returned a valid memory or NULL.
5. Try to write a modular code.