pointer being realloc'd was not allocated?

Question

Maybe it's a stupid question, but I get stuck here for a while.

Let's say freq_tostring() converts a word frequency freq into string, and freq_intostream() appends that string to the end of a stream.

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

typedef struct {
    char *word; // null-terminated
    int freq;
} freq;

/**
 * Constructor
 */
void new_freq(freq *fq, const char *word, const int freq) {
    fq->word = (char *)malloc((strlen(word) + 1) * sizeof(char)); // +1 for null-terminator
    strcpy(fq->word, word);
    fq->freq = freq;
}

/**
 * Free memory
 */
void dispose_freq(void *fq) {
    freq *p = (freq *)fq;
    free(p->word);
    p->word = NULL;
}

/**
 * snprintf() will terminate the string with a null character, unless buf_size is zero.
 */
char *freq_tostring(freq *fq) {
    size_t wordlen = strlen(fq->word);
    char *buffer = (char *)malloc(wordlen + 16); // maximum integer has 10 digits
    snprintf(buffer, wordlen + 16, "[%s, %d]\n", fq->word, fq->freq);
    return buffer;
}

/**
 * Append the string of freq to the end of stream.
 */
void freq_intostream(void *elem, void *stream) {
    freq *fq = (freq *)elem;
    char *str = *(char **)stream;
    size_t strsize = strlen(str);
    // printf("Stream = \"%s\", length = %lu\n", str, strsize);
    char *word = freq_tostring(fq);
    size_t wordsize = strlen(word);
    // printf("Element = \"%s\"%lu\n", word, wordsize);
    char *temp = (char *)realloc(str, strsize + wordsize + 1);
    strcpy(temp + strsize, word);
    temp[strsize + wordsize] = '\0';
    // printf("After strcpy(): \"%s\"\n", temp);
    str = temp;
    free(word);
}


int main(void) {
    freq apple, banana, kiwi;
    new_freq(&apple, "apple", 3);
    new_freq(&banana, "banana", 2);
    new_freq(&kiwi, "kiwi", 5);

    char *buffer = (char *)malloc(1);
    buffer[0] = '\0';
    freq_intostream(&apple, &buffer);
    freq_intostream(&banana, &buffer);
    freq_intostream(&kiwi, &buffer);

    assert(strlen(buffer) == 33); 
    assert(strcmp(buffer, "[apple, 3]\n[banana, 2]\n[kiwi, 5]\n") == 0);

    dispose_freq(&apple);
    dispose_freq(&banana);
    dispose_freq(&kiwi);
    free(buffer);
}

The weird thing is, when I run 10 times, it gives me about 9 pointer being realloc'd was not allocated, but maybe in 1~2 cases, everything is ok.

If I comment out the printf(), it shows that before appending the third element kiwi, the stream is empty, and that could be why realloc is failed. But I'm sure that I pass a pointer of char * stream to the freq_intostream() function, which is a char ** for sure. I can't find out what's the problem, anyone can help?

Answer 1

You've done the equivalent of i = j; i = 3; when you wanted j = 3;. Obviously, these don't do the same thing. Have a close look at the marked line in this funciton:

/**
 * Append the string of freq to the end of stream.
 */
void freq_intostream(void *elem, void *stream) {
    freq *fq = (freq *)elem;
    char *str = *(char **)stream;
    size_t strsize = strlen(str);
    // printf("Stream = \"%s\", length = %lu\n", str, strsize);
    char *word = freq_tostring(fq);
    size_t wordsize = strlen(word);
    // printf("Element = \"%s\"%lu\n", word, wordsize);
    char *temp = (char *)realloc(str, strsize + wordsize + 1);
    strcpy(temp + strsize, word);
    temp[strsize + wordsize] = '\0';
    // printf("After strcpy(): \"%s\"\n", temp);
    str = temp; // OOPS!!
    free(word);
}

You change the value of str, but str is a local to this function and its value is thrown away as soon as the function ends.

You wanted: *(char**)stream = temp; to change the value the caller passed you a pointer to.

This code would be much simpler if you get rid of all the casts. If elem were of type char **, you could just do *elem = temp; and the code would be much easier to understand.