0
struct table
  {
    int size;
    key_value *array[];
  };

struct table *create_table(int size)
{
    struct table *table = calloc(1, sizeof(struct table));
    table->size = size;
    return table;
}

void resize_table(struct table *table, int new_size)
{
    struct table *new_table = create_table(new_size);
    for(int i = 0; i < table->size; i++)
      {
      key_value *p = table->array[i]->next;
      while(has_next(p))
        {
        insert(new_table, p->key, p->value);
        p = p->next;
        }
      }
    *table = *new_table;
}

I tried making the array only array[], this didn't work either. When I try to get rehashed values through new_table, it works perfectly. But after assigning *table = *new_table and accessing rehashed values through table, it doesn't work. Why does assert(new_table->array[23]->key==23) work, but not assert(table->array[23]->key==23)? I have put table equal to new_table, shouldn't they be the same now? What's weird is that the size value has changed, but not the array.

When I try to change just the pointer:

table->array = new_table->array;

I get invalid use of flexible array member, which I don't understand, aren't I just changing a pointer in the struct to a different adress, why does it even matter that it's an array?

Can I solve this by using alloc/realloc maybe?

Blue
  • 51
  • 4
  • 1
    You should show us how you create the hashtable or insert into it. Also how do you call the function you showed? Please provide a [MCVE](https://stackoverflow.com/help/mcve). – Gerhardh Oct 20 '21 at 09:37
  • You can never assign structures holding a flexible array member to another variable. The compiler does not know the size of your array. You can only do this using dynamic memory allocation and manually keeping track of the sizes. – Gerhardh Oct 20 '21 at 09:39
  • Also: You can never assign an array to another array. You can only initialize arrays or copy members of arrays. "When I try to change just the pointer:" `array` is not a pointer but an arrays of pointers. – Gerhardh Oct 20 '21 at 09:40
  • 2
    Why are you making up the code in your question, instead of just _pasting_ the minimal verifiable example you already have? And why are you describing a compiler error instead of just _pasting_ it and showing which line it refers to? – Useless Oct 20 '21 at 09:43
  • 1
    `*table = *new_table` will only copy the `size` member and any padding before the `array` member. It will not copy the flexible array member. – Ian Abbott Oct 20 '21 at 09:43
  • 1
    Stop providing code that you retyped but doesn't resemble what you are actually compiling. Show some real code with real error messages. – Gerhardh Oct 20 '21 at 09:43
  • "Can I solve this by using alloc/realloc maybe?" Unless that's what `create_table` is already doing, you have some massive bugs all over the place. Please post the code for that function. – Lundin Oct 20 '21 at 09:44
  • Create_table uses malloc, can I use realloc to increase the array size somehow? – Blue Oct 20 '21 at 09:48
  • Yes, no, maybe. Since you don't appear to know what a flexible array member is, then I have reason to believe that `create_table` isn't correctly implemented (either). – Lundin Oct 20 '21 at 09:48
  • Quote: **"aren't I just changing a pointer in the struct to a different adress"** NO, because there is no pointer in the struct! What you have is a flexible array. – Support Ukraine Oct 20 '21 at 09:51
  • Most likely you want `key_value *array[];` --> `key_value **array;` – Support Ukraine Oct 20 '21 at 09:52
  • @4386427 Not necessarily. An array of pointers makes perfect sense for a hash table implementing "chaining" (a linked list of items per duplicate hash index). A pointer-to-pointer not so much, unless it points to the first pointer in an array of pointers. – Lundin Oct 20 '21 at 09:54
  • Ok I'll vote re-open and try to answer after last edit. (My dupehammer clicked apparently, gonna need 2 more votes...) – Lundin Oct 20 '21 at 09:58
  • `table->size = size;` Is `size` meant to indicate the size of the array? You have memory for 0 elements allocated, no matter what `size` is. – Gerhardh Oct 20 '21 at 10:00
  • 1
    In case it doesn't get re-opened, then as I predicted by "gut debugging", `create_table` is all wrong too. This whole code needs to be rewritten. – Lundin Oct 20 '21 at 10:00
  • @Lundin From both the posted code and text, it seems that OP doesn't know the flexible array concept but thinks of `array` as a pointer. Therefore I assume OP wants a pointer. – Support Ukraine Oct 20 '21 at 10:03
  • @Lundin There is still much code missing. And we don't know what OP is trying. So perhaps it's too early to reopen – Support Ukraine Oct 20 '21 at 10:05
  • There are several problems here: when it is appropriate to hard-copy a struct and when it is not. How to copy arrays in C. How flexible array members work. How passing dynamically allocated data back from a function. And so on. I could write an answer addressing at least these issues, though there may be more of them. – Lundin Oct 20 '21 at 10:07
  • Sounds good, my program worked well before, when the table was fixed (array[24]), C-unit tests & valgrind no leaks. Just having problems with making it dynamic right now which is tied to me not understanding those things you listed. – Blue Oct 20 '21 at 10:10
  • `create_table` is not allocating any space for the flexible array member. Is it meant to? If it is supposed to allocate space for `size` elements, then it should be something like `struct table *table = malloc(sizeof(struct table) + size * sizeof (key_value));`. Then I guess you need a loop to set the array elements to null pointers. – Ian Abbott Oct 20 '21 at 10:15
  • 2
    @IanAbbott No, it requires `sizeof (key_value*)` – Support Ukraine Oct 20 '21 at 10:16
  • @4386427 Yes, you are correct. Sorry! – Ian Abbott Oct 20 '21 at 10:21
  • Either the first parameter of `resize_table` should be of type `struct table **` or it should return a pointer to the new `struct table`. – Ian Abbott Oct 20 '21 at 10:25
  • The part inside `resize_table`'s `for` loop where it is transferring key-value pairs from the old table to the new table looks like it is skipping the first and last elements of the list. – Ian Abbott Oct 20 '21 at 10:29

3 Answers3

1

There are a lot of misconceptions here.

  • key_value *array[]; is a so-called flexible array member, a special feature. Specifically it is an array of key_value pointers, of a size unknown at the point of declaration. It is an array, not a pointer.

    Therefore table->array = new_table->array; tries to assign an array to another array, which isn't allowed in C. Arrays must be copied with memcpy or similar.

  • Flexible array members mostly make sense when you need to allocate adjacent memory directly following the struct. A hash table isn't an obvious use-case for such, but it can be made to work (and gives good cache performance). What you need to do when allocating a flex array member is to allocate room for the struct and the array in one go, like this:

    struct table *table = calloc(1, sizeof(struct table) + sizeof(key_value*[size]) );
    

    A special attribute of flexible array members is that sizeof(struct table) doesn't take the flex array in account. So we need to specify that size separately.

    If you don't do this, then key_value isn't valid memory and can't be used.

    calloc can be assumed to set all pointers to null, so that's good, keep calloc instead of malloc which doesn't init anything.

  • This here won't work:

    void resize_table(struct table *table, int new_size)
    {
      struct table *new_table = create_table(new_size);
    

    For the reasons explained at Dynamic memory access only works inside function

    There are two ways you could make this work:

    • Either table is assumed to point at previously allocated data. In that case you should not call create_table but modify the already allocated data. Possibly realloc it. In which case realloc must look use sizeof(struct table) + sizeof(key_value*[new_size]).

      realloc, unlike calloc, does not initialize the data to zero, so you must do that explicitly for the new items if you use realloc.

    • Or alternatively you can ask the caller to provide you with an address to the previously allocated data, through struct table** table. Then you can free it and then just call create_table anew, storing the result inside *table. Naturally you might want to memcpy over any items that should be preserved before calling free though.

  • Similarly, *table = *new_table; won't work since that will not copy the flexible array member, nor any data pointed at by the pointers it contains.

    But in general one cannot hard copy a struct when it has pointer members to dynamically allocated data, or you end up with two structs where the members point at the same data. This means that you would have to allocate all data anew using either of the two methods I described above.

Note that all of the above merely allocates room for the pointers themselves in key_value *array[]. They have to be set to point at actual data allocated elsewhere, outside this struct.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • If I go the flexible array member route, can I use realloc to increase the size on the array afterwards? – Blue Oct 21 '21 at 08:42
  • @Blue Yes, that's what the answer says - you must realloc everything including the struct. – Lundin Oct 21 '21 at 09:24
0

When you originally had

struct table
  {
    int size;
    key_value *array[24];
  };

(or whatever), the fixed-size 24-element array is part of your struct, and is counted in sizeof(struct table), and space is allocated for it correctly in

struct table *create_table(int size)
{
    struct table *table = calloc(1, sizeof(struct table));
    table->size = size;
    return table;
}

But when you change this to a flexible array, it has no default size, and no storage for it is assumed in sizeof(struct table). This should be obvious, because you've explicitly told the compiler you don't know how big it should be. You could easily confirm your understanding here by just printing sizeof the two versions of your struct.

Therefore, you need to explicitly tell calloc how many extra bytes to allocate:

    struct table *table = calloc(1, sizeof(struct table) + size * sizeof(key_value*));

Finally, the assignment

  *table = *new_table;

won't work either, because the two objects are not necessarily the same size. You can't resize the first object (whose array length the compiler doesn't even know) like this, and no elements will be copied from the flexible array either (because the compiler doesn't know how many there are).

Now, you already allocated an object of the correct size - You need to return this to the caller. The usual approach would be to pass a pointer-to-pointer, so you can update the caller with the new address:

void resize_table(struct table **table_ptr, int new_size)
{
    struct table *table = *table_ptr; /* this is just to avoid rewriting the rest of the code */
    struct table *new_table = create_table(new_size);
    for(int i = 0; i < table->size; i++)
      {
      key_value *p = table->array[i]->next;
      while(has_next(p))
        {
        insert(new_table, p->key, p->value);
        p = p->next;
        }
      }
    *table_ptr = new_table;
    free(table); /* you're replacing the table, not updating it */
}

There are potentially other problems, like the fact that you seem to always skip the first element in a bucket, and you could use realloc to reduce the number of redundant calloc/free operations.

Useless
  • 64,155
  • 6
  • 88
  • 132
0

Since array inside the struct is without a size specification, it's a flexible array member and it needs dynamic memory allocation. Your code never do that so your array has no memory. You can implement it using a flexible array but for this I would go for a pointer to pointer to key_value.

Like:

struct table
{
    int size;
    key_value **array;
};

struct table *create_table(int size)
{
    struct table *table = malloc(sizeof *table);
    assert(table != NULL);
    table->size = size;
    table->array = calloc(size, sizeof *table->array);
    assert(table->array != NULL);
    return table;
}

void resize_table(struct table *table, int new_size)
{
    key_value **tmp = realloc(table->array, new_size * sizeof *table->array);
    if (tmp == NULL)
    {
        // Error handling or just:
        exit(1);
    }
    table->size = new_size;
    table->array = tmp;
}

If you prefer a solution using a flexible array member, it would something like:

struct table
{
    int size;
    key_value *array[];
};

struct table *create_table(int size)
{
    struct table *table = calloc(1, sizeof *table + size * sizeof table->array[0]);
    assert(table != NULL);
    table->size = size;
    return table;
}

struct table * resize_table(struct table *table, int new_size)
{
    struct table *tmp = realloc(table, sizeof *table + new_size * sizeof table->array[0]);
    if (tmp == NULL)
    {
        // Error handling or just:
        exit(1);
    }
    tmp->size = new_size;
    return tmp;
}

and use the resize_table like:

table = resize_table(table, new_size);
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63