C Program Issue "pointer being freed was not allocated"

Question

I guess my C is a little bit rusty now because I can't quite figure out the issue here. I'm pretty sure it lies within parse_historical_data(). If I comment it out and just run allocate_historical_data() and free_historical_data() then the program runs just fine, but when I uncomment it and run the whole thing I get an error that says "pointer being freed was not allocated".

struct historical_data {
    char **timestamp;
    float *close;
    float *high;
    float *low;
    float *open;
    int *volume;
    int count;
};

struct historical_data *h = (struct historical_data *) malloc(
         sizeof(struct historical_data));


void allocate_historical_data(struct historical_data *h,
         int days, int interval)
{
    int i;

    h->close = malloc(days * (60/interval) * 400 * sizeof(float));
    h->high = malloc(days * (60/interval) * 400 * sizeof(float));
    h->low = malloc(days * (60/interval) * 400 * sizeof(float));
    h->open = malloc(days * (60/interval) * 400 * sizeof(float));

    h->timestamp = (char **) malloc(days * (60/interval)
                               * 400 * sizeof(char *));

    for (i = 0; i < days * (60/interval) * 400; i++)
        h->timestamp[i] = malloc(25 * sizeof(char));

    h->volume = malloc(days * (60/interval) * 400 * sizeof(int));
}


void parse_historical_data(struct historical_data *h, char *raw_data)
{
    char *csv_value;
    int i = 0;

    csv_value = strtok(raw_data, ",");
    h->timestamp[i] = csv_value;

    while (csv_value != NULL) {
        csv_value = strtok(NULL, ",");
        h->open[i] = atof(csv_value);

        csv_value = strtok(NULL, ",");
        h->high[i] = atof(csv_value);

        csv_value = strtok(NULL, ",");
        h->low[i] = atof(csv_value);

        csv_value = strtok(NULL, ",");
        h->close[i] = atof(csv_value);

        csv_value = strtok(NULL, "\n");
        h->volume[i] = atoi(csv_value);

        if (h->volume[i] == 0) // Junk data.
            i--;

        i++;

        csv_value = strtok(NULL, ",");
        h->timestamp[i] = csv_value;
    } 

    h->count = i;
}


void free_historical_data(struct historical_data *h, int days, int interval)
{
    int i;

    free(h->close);
    free(h->high);
    free(h->low);
    free(h->open);

    for (i = 0; i < days * (60/interval) * 400; i++)
        free(h->timestamp[i]);

    free(h->timestamp);
    free(h->volume);

    free(h);
}

**[Do NOT cast the return value of `malloc()`!](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858)** — , Jul 27 '13 at 06:48
I've removed the casts on the return value of malloc() from my code. — user42946, Jul 27 '13 at 06:57
I wish the 'do NOT cast the return value of `malloc()`' crowd would get off their high horse. — Jonathan Leffler, Jul 27 '13 at 07:04
It's preferable not to cast it, as casting it can hide bugs. No high horse there. — Jim Balter, Jul 27 '13 at 08:02
@JimBalter Yeah. There's a difference between saying "[In C, you shouldn't cast the return value of `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858), and shouting "**[Do NOT cast the return value of `malloc()`!](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858)**". — This isn't my real name, Aug 06 '13 at 01:02
Not a difference relevant to Jonathan's comment. If he were simply complaining about shouting, he would have said that. — Jim Balter, Aug 06 '13 at 02:29

Rohan · Accepted Answer · 2013-07-27T07:07:16.443

1

I think the problem lies with h->timestamp.

In parse_historical_data() you do

csv_value = strtok(raw_data, ",");
h->timestamp[i] = csv_value;
...
...
    csv_value = strtok(NULL, ",");
    h->timestamp[i] = csv_value;

h->timestamp[i] is not assigned with allocated string/pointer. Rather it just points to same string - char * but from different index.

You may want to change it too

strcpy(h->timestamp[i], csv_value); //as you have already allocated for it

edited Jul 27 '13 at 07:07

answered Jul 27 '13 at 06:41

Rohan

52,392
12
90
87

1

Unfortunately, this causes me to receive a segmentation fault. – user42946 Jul 27 '13 at 06:45
The diagnosis of the problem is very probably right; I'm not convinced about the remedy because using `strdup()` leaks the memory allocated in `allocate_historical_data()`. It looks like you should be using `strcpy(h->timestamp[i], csv_value);` to me. Otherwise, you should be error checking the values from `strtok()` before using the pointer. – Jonathan Leffler Jul 27 '13 at 07:02
That's likely because you're copying from a null pointer or copying to a null pointer. Since there's no error checking in the code, it is hard to say which is more likely. However, I think that if it was 'working' apart from the complaint from `free()`, then it is more likely that that `csv_value` is OK but some `h->timestamp[i]` is null when you'd rather it wasn't than vice versa. – Jonathan Leffler Jul 27 '13 at 07:05
Thank you sir. This fixed the problem... if (csv_value != NULL) strcpy(h->timestamp[i], csv_value); I should have been error checking. – user42946 Jul 27 '13 at 07:10

C Program Issue "pointer being freed was not allocated"

1 Answers1