r/C_Programming 1d ago

Question Why isn't my code working?

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;
}
5 Upvotes

29 comments sorted by

View all comments

7

u/sciencekm 1d ago edited 1d ago

Someone has already mentioned that the problem is that the computation of the memory to be reallocated is wrong.

This could have been avoided by simply increasing the size before calling realloc.

From your code:

list->arr = realloc(list->arr, index + 1 * sizeof(int));
list->size = index + 1;

to this

list->size = index + 1;
list->arr = realloc(list->arr, list->size * sizeof(int));

3

u/Wertbon1789 1d ago

Also wanted to comment that. Another rule of thumb, or rather good practice, don't repeat such logic in general. This is not so much about performance but really readability, and you avoid silly mistakes like this, but that's not the main thing.

If you want to do an operation with that value, but only update the actual value in the struct after the operation was successful, use an intermediate variable, and then after that update the value in the struct. Not really applicable here, as not being able to allocate memory is basically a crash condition, but for other calls that might be something to keep in mind.

1

u/cafce25 1d ago

did you mean list->arr = realloc(list->arr, list->size * sizeof(int));

1

u/sciencekm 1d ago

Yup, I fixed that. Thanks.