0

I'm writing a function in C which adds a new product entry to an array of structs. The array is defined in the following structure:

struct product_array {
    struct product *arr;
    unsigned int count; //Initially set to NULL, counts the number of entries
    };

The array is dynamically allocated, but each time a new entry is added it should be reallocated. Each element of an array consists of product structure:

struct product {
    char *title;  
    char code[8]; // Should be truncated to 7 characters
   };

This is what I wrote:

 void add_product(struct product_array *pa, const char *title, const char *code)


    {
        pa->arr = realloc(pa->arr, sizeof(struct product) * (pa->count + 1));
        if (pa->arr == NULL)
            return NULL;

        char *temp = malloc(strlen(title) + 1); // for title memory 
//should be dynamically allocated separately
        if (temp == NULL){
            return NULL;
        }

        memmove(temp, title, (strlen(title) + 1));
            title = temp;


        pa->arr[pa->count].title = title;

        int j = 0;
            while (*code) {
                pa->arr[pa->count].code[j] = (*code);
                code++;
                j++;
                if (j == 7)
                    break;
            }
        pa->arr[pa->count].code[j] = '\0';

        pa->count++;

     } 

It seems to work fine (though I'm not sure if I used realloc correctly). But now I'm supposed to release the memory. I did it by writing:

free(pa->arr);

It also seems to be okay. But now I'm supposed to release memory which was allocated for title. To do that I should change main.c.

int main()
{
    struct product_array pa;
    pa.count = 0;
    pa.arr = NULL;

    add_product(&pa, "Product 1", "1111");

    add_product(&pa, "Product 2", "123320");

    add_product(&pa, "Product 3", "565496845");

}

And here I'm lost.

free (pa.arr->title);

doesn't seem to work.

Would really appreciate your help.

1
  • You already have answers, but a couple of tips to add: (1) memcpy is preferred over memmove except when the two memory areas may overlap. However, (2) using memmove (or memcpy) as you are: "memmove(dest, src, strlen(src) + 1)" is essentially identical to "strcpy(dest, src)"; the latter would be more intuitive to the reader of your code. Commented Apr 7, 2015 at 12:45

3 Answers 3

1

Before freeing the pa->arr, you have to iterate over the array, freeing each structure's title separately, like this:

   for (int i = 0; i < pa.count; ++i)
   {
      free(pa.arr[i].title);
   }
   free(pa.arr);
Sign up to request clarification or add additional context in comments.

Comments

0

There are many problems there.

Be careful when using realloc: If you call realloc() and there is not enough room to enlarge the memory allocation pointed out it will copy everything to a new position, what may be slow. So, if the array is something that you expect to change it's size a lot, and you are not working in a platform that has tight memory size restrictions, you should do it in a different way. For example: allocate a space for 500 entries, and when it gets full, call a realloc to grow it to 1000 entries. This way you save some processing power (and memory bandwidth) by using more memory.

Also, make sure you understand how memory allocation works. A good reference is http://www-bcf.usc.edu/~dkempe/CS104/08-29.pdf

For example, if you call:

free(pa->arr);

Just the array will be freed, all the titles will be still allocated in memory, but you lost all the references to them. This is called memory leak (http://en.wikipedia.org/wiki/Memory_leak).

Comments

0

In your main() class, you allocated 3 different instances of 'title', when you go to free the memory, you need to free 3 different instances of 'title'. The simple answer is to just loop through pa->arr and free the title from each member.

One trick I used when writing 'C' is to create a function that deallocates the members of a struct. Then if you are allocating in an array like above, you just loop through the array calling the free_() function to free all the members of the struct.

void free_product(struct product *p) {
  free (p->title);
  /* If you dynamically allocate more fields in p, free them here */
}

void free_product_array(struct product_array *pa)
{
  int i;
  for (i = 0 ; i < pa->count; i++) {
    free_product(&pa->arr[i]);
  }
  free (pa->arr);
}

Your case is a bit different that what I normally do. I usually free the pointer itself at the end of the function, but I wouldn't do that here because you allocated an entire array for your product entries at once.

2 Comments

maybe you meant:free_product_array_item(&pa->arr[i]);
Thanks @Giorgi, I should have run the free_* code through the compiler first!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.