1

I am trying to add elements to a dynamic array of structures in a function. Everything seems to be correct and when I add the first element it actually succeeds but when I try to add another I get:

realloc(): invalid next size

I tried to debug it in many ways, checking if all of the values are correct - the current size, the allocated memory and so on and everything is as it should be. I even did it "manually" (without a function) and it worked as it is supposed to work but when I do it with the function it fails. Is there a mistake I have made in my code which I do not see or am I missing something about allocating dynamic memory?

Here is the code:

typedef struct Product_t {
    char name[100];
    int price;
    int id;
} Product;

void add_product(Product** products, Product product, int *size) {
    (*size) += 1;
    
    if(realloc((*products), (*size) * sizeof(Product)) == NULL)
        exit(1);
        
    (*products)[*size - 1] = product;
}

int main() {

    Product* products = (Product*) malloc(0);
    int products_size = 0;
    
    Product p1 = {"product1", 45, 1};
    Product p2 = {"product2", 212, 2};
    Product p3 = {"product3", 123, 3};
    
    add_product(&products, p1, &products_size);
    add_product(&products, p2, &products_size);
    add_product(&products, p3, &products_size);
    
    return 0;
}

P.S. I know it would be better to do this with a linked list for example but I am required to do it with a dynamic array.

gluhtuten
  • 23
  • 5
  • 6
    You're throwing away the result of `realloc`. You need to assign it to a variable, and (if it's not `NULL`) assign it to `*products`. (The issue is that `realloc` can move the allocated memory, so you need to update your pointer to point to the new location; the old location is then no longer allocated memory.) – psmears May 23 '23 at 14:04
  • For correct usage of `realloc`, see here: https://stackoverflow.com/questions/21006707/proper-usage-of-realloc – nielsen May 23 '23 at 14:09
  • After calling `realloc`, the original pointer value is no longer value, and you're discarding the new pointer value, so at that point you're lost. You need to save the pointer returned by `realloc`, and use it as the new address of your array. If you think about it, it should be obvious why it has to work this way. – Tom Karzes May 23 '23 at 14:12
  • Basically: `if (realloc((*products), (*size) * sizeof(Product)) == NULL)` -> `if ((*products = realloc((*products), (*size) * sizeof(Product))) == NULL)` – Jabberwocky May 23 '23 at 14:18
  • If it's available to you, use [Valgrind](https://valgrind.org/) to help diagnose the problems. If you're using GCC, consider using the address sanitizer. Either or both will help you discover where you're treading outside the bounds of your memory. – Jonathan Leffler May 23 '23 at 14:31
  • 1
    `malloc(0);` is brittle and poorly-defined. Instead just initialize the pointer to NULL, then you can pass it safety to realloc. – Lundin May 23 '23 at 14:33
  • 1
    @Lundin: It's defined to either return NULL or a unique pointer that can be passed to `free()`. Either option works fine in this case. – psmears May 23 '23 at 15:02
  • @psmears No guarantees. "or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object". That's all it says. Compilers never implemented that consistently or portably. The bottom line is: don't run off to write weird code just for the heck of it. – Lundin May 23 '23 at 16:33
  • No, there _are_ the specific guarantees that I mentioned. Yeah, you can't _write_ via the pointer, but this code doesn't, nor does it assume anything about the pointer's NULL-ness (which is the other area of unsafety). – psmears May 23 '23 at 16:54
  • @TomKarzes @ nielsen @ psmears @ Jabberwocky Thank you it worked that way! – gluhtuten May 23 '23 at 18:02
  • @JonathanLeffler I ran Valgrind but did not find the output helpful, I already assumed there was something wrong with the `realloc()`, maybe I do not know how to use it properly so it helps me find what's wrong. – gluhtuten May 23 '23 at 18:05
  • The key point with Valgrind is to compile with debugging enabled (if you use GCC or Clang, make sure you include `-g` both when creating object files and when linking the program). You will then be told the function, source file, line number where the problem occurs and the stack trace of functions leading to the troublesome spot. Omit the debugging information and you get very little useful information. – Jonathan Leffler May 23 '23 at 18:57

1 Answers1

2

Address pointed by the pointer passed to the realloc is not guaranteed to remain the same, per man page of the realloc. So you need always assign the return value of realloc(...) to (*products) unless it is NULL.

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
no more sigsegv
  • 468
  • 5
  • 17
  • Generally, I do not think that it is a good idea to post links to platform-specific documentation, if the question does not apply to a specific platform. The reason why I believe this is generally a bad idea is that a particular platform may provide additional guarantees that the ISO C standard does not provide. Therefore, posting links to the Linux documentation may be misleading for people using a different platform. For platform-neutral documentation, I recommend [this link](https://en.cppreference.com/w/c/memory/realloc). – Andreas Wenzel May 23 '23 at 15:46
  • Thanks for the heads up, i will be careful @AndreasWenzel – no more sigsegv May 23 '23 at 16:28
  • 1
    Thanks, it worked that way. I think I saw somewhere that it is fine to use `realloc()` the way I guess it is not. I actually forgot that it might change the address pointed by the pointer. – gluhtuten May 23 '23 at 17:20