Search code examples
cmemorycorruption

Memory is corrupted after third call


I am building a rudimentary "banking" system. I am trying to create a function that would generate a new "vault" as i like to call it(account). I also made a sort of interactive shell to send commands to it. However when i try to create the third vault/account, all the memory is corrupted and i can't find where the problem is.

I tried rewriting the code multiple times and re-designing the way the generate function works, however, every time the memory is corrupted after the third insertion. This is my file:

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

struct __attribute__((packed)) Vault {
    unsigned id;
    unsigned pin;
    unsigned balance;
};

struct Vault *GenerateVault(unsigned balance, struct Vault **list, int listLength) {
    struct Vault *vault = (struct Vault *)malloc(sizeof(struct Vault));
    if (vault == NULL) {
        printf("Couldn't allocate memory for new vault!");
        return NULL;
    }

    vault->balance = balance;

    srand(time(NULL));
    vault->pin = arc4random() % (9999+1-1000) + 1000;
    
    int flag = 1;
    vault->id = arc4random() % (999999+1-100000) + 100000;

    do {
        flag = 0;
        for (int i = 0; i < listLength; ++i) {
            if (((*list)[i]).id == vault->id) {
                vault->id = arc4random() % (999999+1-100000) + 100000;
                flag = 1;
                break;
            }
        }
    } while (flag);

    struct Vault *temp = (struct Vault *)realloc(*list, (listLength+1) * sizeof(struct Vault));
    if (temp == NULL) {
        printf("Couldn't reallocate memory for new vault!\n");
        free(vault);
        return NULL;
    }
    *list = temp;

    (*list)[listLength] = *vault;

    return vault;
}

void executeCommand(char *command, struct Vault *list, int *listLength) {
    if (strcmp(command, "generate") == 0) {
        unsigned balance = 100;
        printf("Enter balance: %d\n", balance);

        struct Vault *card = GenerateVault(balance, &list, *listLength);
        if (card != NULL) {
            ++(*listLength);
            printf("Card generated successfully!\n");
            printf("Card id:      %u\n", card->id);
            printf("Card pin:     %u\n", card->pin);
            printf("Card balance: %u\n", card->balance);
        }
    }
}

int main() {
    struct Vault *vaults = NULL;
    int vaultCount = 0;
    char command[256] = "generate";

    vaults = (struct Vault *)malloc(0);

    for (int i = 0; i < 3; ++i) {
        executeCommand(command, vaults, &vaultCount);
    }

    return EXIT_SUCCESS;
}

Solution

  • When you make this call:

    struct Vault *card = GenerateVault(balance, &list, *listLength);
    

    The second parameter is the address of the parameter list passed to the function. This does not affect vaults declared in main. As a result, each time executeCommand is called, you're passing in the same pointer value which was returned from a call to malloc(0). This not only loses data entered on the prior invocation, but on the second and subsequent invocation you read from a pointer value that is no longer valid. This triggers undefined behavior.

    Change the call to pass the address of the allocated list:

    executeCommand(command, &vaults, &vaultCount);
    

    And the definition to accept a variable of this type:

    void executeCommand(char *command, struct Vault **list, int *listLength) {
    

    And change how it's used in the function:

    struct Vault *card = GenerateVault(balance, list, *listLength);
    

    Also, there are a number of memory leaks in the code. You need to free(card) after printing the values, and you need to free(vaults) at the end of main.