Dynamic string array structure in C

Question

I am new to C and I want to create a dynamic array to store strings. I wrote the code below for it but it didn't work. Array elements contain some ASCII chars instead of string. I want historyArray[0] value to be "foo". How can I do that?

typedef struct {
    char *historyCommand;
    int usedSize;
    int maximumSize;
} HistoryArray;

void CreateHistoryArray(HistoryArray *HistoryArray) {
    HistoryArray->historyCommand = (char *) malloc(sizeof(char) * MAX_LEN);
    HistoryArray->usedSize = 0;
    HistoryArray->maximumSize = INITIAL_SIZE;
}

void ExpandHistoryArray(HistoryArray *HistoryArray, int newSize) {
    int *newArray = (char *) malloc(sizeof(char) * newSize);
    memcpy(newArray, HistoryArray->historyCommand, sizeof(char) * HistoryArray->maximumSize);
    free(HistoryArray->historyCommand);
    HistoryArray->historyCommand = newArray;
    HistoryArray->maximumSize = newSize;
}

void AddHistoryValue(HistoryArray *HistoryArray, char historyCommand[]) {
    strcpy(HistoryArray->historyCommand[HistoryArray->usedSize], historyCommand);

    HistoryArray->usedSize++;

    if (HistoryArray->usedSize == HistoryArray->maximumSize) {
        ExpandHistoryArray(HistoryArray, HistoryArray->maximumSize * 2);
    }
}

void freeHistoryArray(HistoryArray *a) {
    free(a->historyCommand);
    a->historyCommand = NULL;
    a->usedSize = 0;
    a->maximumSize = 2;
}


HistoryArray historyArray;

Answer 1

There are a number of problems in your code.

1. char *historyCommand is a pointer to a single string, not an array of strings. For a pointer to an array of strings you should use char **historyCommand.

2. You don't need to allocate the space for the individual strings when you create the HistoryArray. You can allocate the proper amount of space each time you add to the array, using the length of the string you're adding.

3. You should use realloc() instead of calling malloc(), memcpy(), and free(). This has the benefit that sometimes it can simply expand the memory it already allocated, so no copying will be needed.

4. When you're freeing the HistoryArray, you need to free all the strings. You shouldn't free historyCommand, because you set maximumSize = 2, and the other functions assume that this means there's room for 2 items there, which isn't true if you set historyCommand to NULL. So you should resize it to maximumSize to be consistent with the rest of the code.

Here's the new code:

typedef struct {
    char **historyCommand;
    int usedSize;
    int maximumSize;
} HistoryArray;

void CreateHistoryArray(HistoryArray *HistoryArray) {
    HistoryArray->historyCommand = malloc(INITIAL_SIZE * sizeof(char *));
    HistoryArray->usedSize = 0;
    HistoryArray->maximumSize = INITIAL_SIZE;
}

void ExpandHistoryArray(HistoryArray *HistoryArray, int newSize) {
    HistoryArray->historyCommand = realloc(HistoryArray->historyCommand, newSize * sizeof(char *));
    HistoryArray->maximumSize = newSize;
}

void AddHistoryValue(HistoryArray *HistoryArray, char historyCommand[]) {
    historyCommand[HistoryArray->usedSize] = malloc(strlen(historyCommand) + 1);
    strcpy(HistoryArray->historyCommand[HistoryArray->usedSize], historyCommand);

    HistoryArray->usedSize++;

    if (HistoryArray->usedSize == HistoryArray->maximumSize) {
        ExpandHistoryArray(HistoryArray, HistoryArray->maximumSize * 2);
    }
}

void freeHistoryArray(HistoryArray *a) {
    for (int i = 0; i < a->usedSize; i++) {
        free a->historyCommand[i];
    }
    a->usedSize = 0;
    a->maximumSize = 2;
    a->historyCommand = realloc(a->historyCommand, a->maximumSize * sizeof(char *));
}

HistoryArray historyArray;