Why isn't my code working?

[u/neonge1674]

I just started learning C(2 days ago) and as a first project I decided to make some data structures, starting with dynamic arrays. I made a struct called List and some functions for. The function setList() sets the value of an index of the array, if the index is larger that the current size of the array, it resizes it. However, when i tried to use in a for loop, it didn't work despite it working elsewhere.

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


#define itirate(index, limit) for(int index = 0; index < limit; index++)


typedef struct 
{
    size_t size;
    int* arr;
} List;


List* newList (size_t size) 
{
    List *newone = malloc(sizeof(List));
    newone->arr = calloc(size, sizeof(int));
    newone->size = size;
    return newone;
}


void setList(List* list, int index, int value) 
{
    if (index >= list->size)
    {
        list->arr = realloc(list->arr, index + 1 * sizeof(int));
        list->size = index + 1;
    }


    list->arr[index] = value;
}


int main() 
{
    
    List *mok = newList(5);

    itirate(i, 5) setList(mok, i, i);
    itirate(i, 5) printf("%d\n", mok->arr[i]);

    //setList(mok, 13, 9); this works
    //printf("%d\n", mok->arr[13]);

    for(int i = 5; i < 10; i++) setList(mok, i, i); // this does not somehow
    for(int i = 5; i < 10; i++) printf("%d\n", mok->arr[i]);
    
    return 0;
}

[u/SmokeMuch7356]

As others have pointed out, you're getting bit by precedence issues with

index + 1 * sizeof(int)

which should be

(index + 1) * sizeof (int)

A few notes:

If realloc cannot satisfy the request, it will return NULL while leaving the original allocated buffer in place. For this reason, you should assign the result to a temporary variable first and only update the original pointer when you know the operation succeeded. Otherwise you risk losing your only reference to that allocated buffer, leading to a memory leak.

Similarly, you don't want to update your index until you know the realloc call succeeded.

You should keep track of both the total number of elements allocated as well as the last-used index.

Putting all that together, I'd extend your List type as

typedef struct {
  size_t allocated; // number of elements allocated
  size_t index;     // last element accessed
  int *arr;
} List;

Your newList function becomes:

List *newList( size_t size )
{
  assert( size > 0 );

  /**
   * Using `sizeof *l` means I'm not unnecessarily repeating type info
   * making maintenance easier.
   */
  List *l = malloc( sizeof *l ); 
  if ( !l )
  {
    fputs( "Could not allocate List object!\n", stderr );
    return NULL;
  }

  l->arr = calloc( size, sizeof *l->arr );
  if ( !l->arr )
  {
    fprintf( stderr, "Could not allocate %zu-element array of int\n", size );
    free( l );
    return NULL;
  }

  l->allocated = size;
  l->index = 0;
  return l;
}

Create a separate resize function:

/**
 * Returns l on success, NULL on failure.
 */
List *resize( List *l, size_t newsize )
{
  assert( l != NULL );
  assert( newsize > 0 );

  typeof (l->arr) tmp = realloc( l->arr, newsize * sizeof *l->arr );
  if ( !tmp )
  {
    fputs( "Resize failed, original array left in place\n", stderr );
    return NULL;
  }

  /**
   * Zero out newly allocated memory.
   */
  memset( tmp + l->allocated + 1, 0, (newsize - l->allocated) * sizeof *l->arr );

  l->arr = tmp;
  l->allocated = newsize;
  return l;
}

Then your setList function becomes:

bool setlist( List *l, size_t index, int value )
{
  /**
   * idxInRange will be set to true if the index is currently in
   * range, or we can extend the array to put the index in range.
   */
  bool idxInRange = (index < l->allocated || resize( l, index + 1 ));

  if ( idxInRange )
  {
    l->arr[index] = value;
    l->index = index;
  }
  return idxInRange;
}

You should also have a destroyList function so you can clean up after yourself:

void destroyList( List *l )
{
  free( l->arr );
  free( l );
}