CODEWITHSUNDEEP
cmalloc
user1986698
user1986698

Reputation: 33

Odd malloc crash

I have the following code that has been working for the last couple of months, but have recently started to crash on occasion (when running in a multi-threaded application):

struct some_struct {
    char* m_str1;
    char* m_str2;
}

struct some_struct*
set_some_struct(const char* p_str1, const char* p_str2) {
    struct some_struct* some_struct_ptr = 
        (struct some_struct*)malloc(sizeof(struct some_struct));
    if (some_struct_ptr == NULL)
        printf("malloc failed!\n");

    size_t str1_len = strlen(p_str1) + 1;
    size_t str2_len = strlen(p_str2) + 1;

    some_struct_ptr->m_str1 = malloc(str1_len);
    if (some_struct_ptr->m_str1 == NULL)
        printf("malloc failed!\n");

    some_struct_ptr->m_str2 = malloc(str2_len); // Crashes here
    if (some_struct_ptr->m_str2 == NULL)
        printf("malloc failed!\n");

    strcpy(some_struct_ptr->m_str1, p_str1);
    strcpy(some_struct_ptr->m_str2, p_str2);

    return some_struct_ptr;
}

Running it gives me "The instruction at "0x7c81bb52" referenced memory at "0x00000002". The memory could not be "read"."

Is there anything obviously wrong with the code above that could have it misbehave under certain circumstances? If I run the function alone in a test program it works just fine, but it always crashes when running in the full application. Everything leading up to the third malloc seems just fine.

EDIT: Further investigation leads me to believe that it is earlier calls to malloc that mess this one up. Is such a thing even possible? If I uncomment a function call being made previous to set_some_struct and that involve several mallocs then set_some_struct will run just fine.

Upvotes: 3

Views: 392

Answers (2)

JimR
JimR

Reputation: 16153

if (some_struct_ptr == NULL)
    printf("malloc failed!\n");

From this point on you're using a garbage pointer. The same problem occurs with the following code.

if (some_struct_ptr->m_str1 == NULL)
    printf("malloc failed!\n");

if (some_struct_ptr->m_str2 == NULL)
    printf("malloc failed!\n");

Upvotes: 1

unwind
unwind

Reputation: 400129

Well, all you do when an allocation fails is print an error; perhaps the printing is dropped or you're missing it? If there are multiple threads running this, output might be confusing.

Second, you're not checking the input pointers. Since the crash is a read, and all other accesses through pointers are writes to your newly allocated area(s), I suspect one or more of the arguments is a NULL pointer. You should check for that.

Also, you shouldn't be casting the return value of malloc() in C (see here for reasons), if you're not including stdlib.h this could be hiding errors.

If the strings are constant, you could save memory and speed by doing just a single call to malloc(), adding up the sizes of the three allocations first and then setting pointers accordingly, of course.

Upvotes: 2

Related Questions