CODEWITHSUNDEEP
c
AppDeveloper
AppDeveloper

Reputation: 2210

c string array with null fails

A method that returns a string array. For some reason this method fails while accessing the string values specifically for the last NULL value.

static char **getList() {
    char **list = (char *[]){"ABC", "DEF", NULL};
    char **interests = malloc(sizeof(char*) * 3);
    int i = 0;
    while(*list) {
        interests[i] = malloc(sizeof(char) * strlen(*list));
        strcpy(interests[i++], *list);
        list++;
    }
    return interests;
}

Using

char **interests = getList();
while(*interests) {
    puts(*interests);
    interests++;
}

Attached screenshot.

enter image description here

Upvotes: 0

Views: 65

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409482

You do some... weird things in the code you show. To begin with we have the list variable, which should be a simple and plain array instead.

And if the array or its contents should not be modified then you could even create an array whose life-time last the full run-time of the program, using static:

static const char * const *getList() {
    static const char * const list[] = { "ABC", "DEF", NULL };
    return list;
}

The declaration const char * const list[] declares list as an array (whose size will be deduced from the initialization) of constant pointers to constant characters. That is, the pointers inside list can't be modified, and the string contents itself can't be modified.

If you have other requirements, like being able to modify the strings, then you could still use the definition shown above in my answer, but create a new array to return.

There are a few improvements that can be made to the allocation and copying code as well. For example you could programatically get the size of the array list. I also recommend that you use a simple index loop instead of the pointer arithmetic loop you have now, and with the correct size of the list array the loop will include copying the terminating NULL as well.

All in all it could look something like

static char **getList() {
    static const char * const list[] = { "ABC", "DEF", NULL };

    // Get the number of elements in the array
    size_t list_size = sizeof list / sizeof list[0];

    // sizeof *interests is the size of each element we want to allocate
    // multiply it by the number of elements we need
    char **interests = malloc(sizeof *interests * list_size);

    // Do a deep copy of the strings from list
    for (size_t i = 0; i < list_size ++i) {
        if (list[i] == NULL) {
            interests[i] = NULL;
        } else {
            interests[i] = malloc(strlen(list[i]) + 1);  // +1 for the string terminator
            strcpy(interests[i], list[i]);

            // Or if your system have the strdup function (which is quite common)
            // interests[i] = strdup(list[i]);
        }
    }

    return interests;
}

If you don't need a deep copy, then the copying loop could be:

for (size_t i = 0; i < list_size ++i) {
    interests[i] = list[i];
}

Upvotes: 0

MikeCAT
MikeCAT

Reputation: 75062

  • Using strlen(), the allocation lack for space for terminating null-character.
  • The last element of what is pointed at by interestes, which should be NULL, is not initialized.

fixed version:

static char **getList() {
    char **list = (char *[]){"ABC", "DEF", NULL};
    char **interests = malloc(sizeof(char*) * 3);
    int i = 0;
    while(*list) {
        interests[i] = malloc(sizeof(char) * (strlen(*list) + 1)); /* allocate one more */
        strcpy(interests[i++], *list);
        list++;
    }
    interests[i] = NULL; /* initialize the last element */
    return interests;
}

Upvotes: 1

Related Questions