0

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;
Nisse Engström
  • 4,738
  • 23
  • 27
  • 42
  • Please provide an [mcve]. How and which functions are you calling? "I want historyArray[0] value is "foo". That doesn't make sense. `historyArray[0]` is not a `char` array. Please be more precise in your description. And please format your code with proper indentation to make it readable. – kaylum Dec 08 '16 at 22:09
  • `char *HistoryArray` is not an array of strings, it's an array of characters, which is just a single string. – Barmar Dec 08 '16 at 22:35
  • `BTW`, `malloc + memcpy + free` is the same as `realloc`. – Barmar Dec 08 '16 at 22:36
  • [don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Dec 08 '16 at 22:36
  • Isn't the compiler warning you about a wrong argument type in the `strcpy()` line? – Barmar Dec 08 '16 at 22:37
  • @Bamar no it's not an array. It's a char pointer – Fredrik Dec 08 '16 at 22:47
  • @Fredrik But since he initializes it with `malloc`, it's a pointer to the array of characters that was allocated. – Barmar Dec 08 '16 at 22:49
  • @Bamar no it's a char pointer to the begining of the memory allocated with malloc. It's not an array. – Fredrik Dec 08 '16 at 22:52

1 Answers1

2

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;
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • The code from [Barmar](https://stackoverflow.com/users/1491895/barmar) gives me a warnings like `warning: assignment to 'char' from 'void *' makes integer from pointer without a cast [-Wint-conversion]`. Try it [here](https://www.mycompiler.io/view/7aSL4jjZf8R) out. – S. Schlegel Jun 14 '22 at 14:43