views:

66

answers:

3

I'm trying to get a sub-string for each member of the struct 'structs' and then assign that sub-string to a new member of the temp_struct. The problem I'm having is how to free the sub-string on each iteration, for some reason the code runs, however valgrind throws an Invalid read of size 1, which I assume I'm reading off the block of memory.

How could I free the sub-string?

Thanks

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

struct st_ex {
    char product[16];
    float price;
};
struct st_temp {
    char *prod;
};

char *temp = NULL;

// from stackoverflow
char* substr( const char* source, size_t start, size_t end )
{
    char* dest = malloc( end - start + 1) ;
    memcpy( dest, &source[start], end - start ) ;
    dest[end - start] = 0 ;
    return dest ;
}

int main()
{
    struct st_ex structs[] = {{"mp3 player", 2.0f}, {"plasma tv", 20.0f},
                              {"notebook", 10.0f},  {"smartphone", 49.9f},
                              {"dvd player", 10.0f}, {"matches", 0.2f }};
    struct st_temp **temp_struct;

    size_t j, i;
    temp_struct = malloc(sizeof *temp_struct * 6);
    for (j = 0; j < 6; j++)
        temp_struct[j] = malloc(sizeof *temp_struct[j]);

    size_t structs_len = sizeof(structs) / sizeof(struct st_ex);

    for(i=0; i<structs_len; i++){
        temp = substr(structs[i].product, 0, 4);
        temp_struct[i]->prod = temp;
        free(temp);
        temp = NULL;
    }
    for(i=0; i<6; i++ )
        printf("%s\n",temp_struct[i]->prod);

    for(i=0; i<6; i++ )
        free(temp_struct[i]);

    free(temp_struct);
    return 0;
}
A: 

Don't free substrings. In C, a substring is part of the original string. If you want to obtain a substring independent of a string, use strdup.

mcandre
Josh is malloc'ing memory for the sub string, therefore he needs to free it. If he does use strdup, where should he free the dup'ed string?
Binary Worrier
+1  A: 

1) You are freeing the substring

    temp = substr(structs[i].product, 0, 4); 
    temp_struct[i]->prod = temp; 
    free(temp); 

The third line above frees the memory you malloc'd in substr.

2) because you're freeing the memory here, you've introduced a bug.
It's invalid to access the malloc'd memory after you free it, therefore it's invalid to try to print temp_struct[i]->prod.

The solution?
Don't free(temp), instead in your loop to free temp_struct[i], you first need to free temp_struct[i]->prod, like this

for(i=0; i<6; i++ )     
{
    free(temp_struct[i]->prod);
    free(temp_struct[i]);    
}
Binary Worrier
+1  A: 

Josh, your temp_struct is holding on to the substring... you don't want to free the substring where you currently are freeing it. The invalid read is coming from this line:

for(i=0; i<6; i++ )
    printf("%s\n",temp_struct[i]->prod);

Instead, you want to free the substring when you free your temp structs, like so:

for(i=0; i<6; i++ )
{
    free(temp_struct[i]->prod);
    free(temp_struct[i]);
}
Polaris878