Search code examples
cpointersdynamic-memory-allocation

Array of structs not adding


Hey so we started using C in University a month ago and now we've got an assignment and we need to use dynamic allocation on an array of structs. This is my code for creating the repo: '''

Repo* init() {
    Repo* repo = malloc(sizeof(Repo));
    repo->items = malloc(sizeof(Item) * 1);
    repo->allocatedSlots = 1;
    repo->numberOfItems = 0;
    return repo;
}

typedef struct {
    Item *items;
    int numberOfItems;// the index of the last item
    int allocatedSlots;
}Repo;

''' And these are the functions I am testing and I get the assertion failed: '''

int grow(Repo* repo) {
    assert(repo != NULL);
    assert(repo->items != NULL);
    Item* new_repo = malloc((2 * repo->allocatedSlots)*sizeof(Item));
    if (new_repo == NULL)
        return 0; //allocation fail!
    memcpy(new_repo, repo->items, sizeof(Repo) * repo->numberOfItems);
    free(repo->items);
    repo->items = new_repo;
    repo->allocatedSlots *= 2;
    free(new_repo);
    return 1; //success
}

int addItem(Repo* repo, Item item) {
    for (int index = 0; index < repo->numberOfItems; index++)
        if (repo->items[index].ID == item.ID)
            return 0;// item already exists
    //printf("%d %d", item.ID, item.value);
    if (repo->numberOfItems < repo->allocatedSlots) {
        char* type = (char*)malloc(strlen(item.type)*sizeof(char)), *state = (char*)malloc(strlen(item.state) * sizeof(char));
        strcpy(type, item.type);
        strcpy(state, item.state);
        //repo->items[repo->numberOfItems + 1] = &item;
        repo->items[repo->numberOfItems + 1].ID = (int)item.ID;
        //printf("JSJSJSJS  %d", repo->items[repo->numberOfItems + 1].ID);
        repo->items[repo->numberOfItems + 1].value = item.value;
        strcpy(repo->items[repo->numberOfItems + 1].state, state);
        strcpy(repo->items[repo->numberOfItems + 1].type, type);
        repo->numberOfItems++;
    }
    else {
        grow(repo);
        repo->items[repo->numberOfItems + 1].ID = item.ID;
        repo->items[repo->numberOfItems + 1].value = item.value;
        strcpy(repo->items[repo->numberOfItems + 1].state, item.state);
        strcpy(repo->items[repo->numberOfItems + 1].type, item.type);
        repo->numberOfItems++;
    }
    return 1;//adding successfull!
}

void AddItem_ValidInput_AddedToList() {
    Repo* repo = init();

    Item item ;
    int id = 1, value = 100;
    char* state = "New";
    char* type = "Uglyo";
    item = createItem(id, value, state, type);
    addItem(repo, item);
    //printf("sdfsdfsdfs");
    //printf('%d\n', repo->items[0].ID);
    assert(repo->items[0].ID == item.ID);
    //printf('%d', item->ID);
    destroy(repo);

}

''' The last one is the test case I am doing first.


Solution

  • The problem is most likely here:

    char* type = (char*)malloc(strlen(item.type)*sizeof(char)), *state = (char*)malloc(strlen(item.state) * sizeof(char));
    strcpy(type, item.type);
    strcpy(state, item.state);
    

    Here you forget that char strings in C are really called null-terminated byte strings. A string in C is a sequence of characters, terminated by a zero. This zero is not counted by strlen, but it still needs space allocated for it.

    You need to allocate strlen(...) + 1 bytes for the strings:

    char* type = malloc(strlen(item.type) + 1);
    char* state = malloc(strlen(item.state) + 1);
    strcpy(type, item.type);
    strcpy(state, item.state);
    

    Or if your system have the strdup function (very likely)

    char* type = strdup(item.type);
    char* state = strdup(item.state);
    

    You also have very serious problems with your grow function:

    Item* new_repo = malloc((2 * repo->allocatedSlots)*sizeof(Item));
    repo->items = new_repo;
    free(new_repo);
    

    First you allocate memory and make new_repo point to that memory.

    Then you make repo->items point to the same memory. You now have two pointers pointing to the exact same memory.

    Then you free that memory, making both the new_repo and the repo->items pointers invalid.

    You should not free(new_repo).

    Or use e.g. realloc instead (as already suggested by me in a comment):

    int grow(Repo* repo) {
        Item* new_repo = realloc(repo->items, 2 * repo->allocatedSlots * sizeof(Item));
        if (new_repo == NULL)
            return 0;  // Failure to allocate
    
        repo->items = new_repo;
        repo->allocatedSlots *= 2;
        return 1;
    }