Unknown Segfault

Question

Thanks in advance for your help. I've done all the research I could trying to debug this. Adding printf's seems to change where the segfault occurs. I'm hardly familiar with gdb but somehow the program ran without issue in it. I've got some quantum observation problem going on. Let's get into the code.

This is just to reacquaint myself with C. The segfault is happening somewhere in create_string(). I could never get a printf() to show up right before the set_string(str, src); call.

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

typedef struct {
    char* value;
    size_t mlength;
    size_t length;
} StringContainer;

typedef struct {
    StringContainer* value;
    StringContainer* next;
} StringContainerList;


void set_string(StringContainer *str, const char* src);
StringContainer* create_string(const size_t max_length, const char* src);
size_t string_length(StringContainer* str);

int main(int argc, char *argv[])
{
    StringContainer* str = create_string(100, "The quick brown fox jumped over the lazy dog.");

    printf("Test: string_length\n");
    printf("%zu\n", string_length(str));
    return 0;
}

StringContainer* create_string(const size_t max_length, const char* src)
{
    StringContainer* str;
    size_t str_len;

    if ( src == NULL )
    {
        str_len = 0;
    }
    else
    {
        str_len = strlen(src);
    }

    str = (StringContainer*)malloc(sizeof(StringContainer));

    str->mlength = max_length;

    if ( str_len > 0 )
    {
        // set_string will delayed-malloc str->value :)
        set_string(str, src);
    }

    return str;
}

void set_string(StringContainer* str, const char* src)
{
    if (str->value == NULL)
    {
        str->value = (char*)malloc(str->mlength * sizeof(char));
        memset(str->value, '\0', str->mlength);
    }

    strcpy(str->value, src);

    str->length = strlen(str->value);
}

size_t string_length(StringContainer* str)
{
    char* value = str->value;
    size_t max_length = str->mlength;
    size_t offset_idx = 0;
    size_t division = max_length / 2;

    while (division != 0)
    {
        if ( value[offset_idx + division] != '\0' )
        {
            offset_idx = offset_idx + division;
        }

        division /= 2;
    }

    return offset_idx;
}

Thanks all for your time. I'm sure it's simple, something possibly fundamental, but my Google-foo is not developed enough to find the root of the issue.

Also, is this design pattern common? Obviously the malloc's should have free's associated. I'll also add in checking the malloc was successful.

`if (str->value == NULL)` -- `str->value` is not initialized. And `str->value = (char*)malloc(str->mlength * sizeof(char));` -- No space for the NUL-terminator. — Spikatrix, Jun 08 '15 at 05:41
Have you tried running in a debugger? Where does it say the crash happens? — Some programmer dude, Jun 08 '15 at 05:41
Oh and the standard [in C you should not cast the return of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) warning. — Some programmer dude, Jun 08 '15 at 05:42
Would you mind if i ask you, why did you (tried to) reinvent the whole wheel behind `strlen()`? — Sourav Ghosh, Jun 08 '15 at 05:46
Your `string_length()` function depends on you zeroing the memory in the allocated memory for the string. However, you don't ensure that the memory is zeroed, so it is going to be very unreliable. — Jonathan Leffler, Jun 08 '15 at 05:52
@SouravGhosh - 1) For fun, reacquainting with C as stated above. 2) Given that str was a properly formed string container, string_length should run in O(log n) instead of strlen()'s O(n) — Liandri, Jun 08 '15 at 15:18
@JonathanLeffler indeed you're right. I would need to add the case for when str.value is not-yet alloc'd before jumping in — Liandri, Jun 08 '15 at 15:19

score 2 · Accepted Answer · answered Jun 08 '15 at 05:49

In the create_string() function you should properly set the values of malloced str so that it does not contain any random values.

Especially, str->value is not set to NULL. And then you are checking it in set_string() which may not succeed and you would strcpy() to unallocated memory causing undefined behavior.

So for minimum update your create_string() function as

str = (StringContainer*)malloc(sizeof(StringContainer));

str->mlength = max_length;
str->value = NULL; //add this

And indeed that works! This is something that is now obvious to me, but I needed the extra set of eyes (or fifty :P) to pick it out. Thanks, all. — Liandri, Jun 08 '15 at 15:25

Unknown Segfault

1 Answers1