Passing an array of strings as argument and modifying it dynamically within the function in C-CodePudding

I need to dynamically populate an array of strings in C. Here is the example of my code. Compiles fine, segmentation faults. Is it possible to achieve this functionality?

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

#define MAX_FILE_NAME_LENGTH 255

void loadList(char*** files,int *filesSize);

int main (int argc, char *argv[])
{
  char** files;
  int filesSize=-1;

  loadList(&files, &filesSize);
  //printf("filesize= %d\n",filesSize );
  for(int i=0;i< filesSize;i  ) printf("%s\n", files[i]);
  free(files);

  return 0;
}

////////////////////////////////////////////////////////////////////////////////

void loadList(char*** files,int *filesSize){
 *filesSize=3;
 //some arry for the example
 char *values[] = {"100.dat", "150.dat", "200.dat"};
 for(int i=0;i< *filesSize;i  ){
   *files=(char**) realloc(*files,(i 1) * sizeof(**files));
   *files[i]=malloc((MAX_FILE_NAME_LENGTH 1) * sizeof(char*));
   strcpy(*files[i],values[i]);
   printf("%s\n", files[i]);
 }
}

CodePudding user response：

In *files = (char **)realloc(*files, (i 1) * sizeof(**files)); you rely on *files to point at already allocated memory or NULL. Neither is true.

*files[i] should also be (*files)[i] because of operator precedence.

You also forgot to free the allocations made inside the loop. A fix could look like this:

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

#define MAX_FILE_NAME_LENGTH 255

void loadList(char ***files, size_t *filesSize) {
    // some array for the example
    const char *values[] = {"100.dat", "150.dat", "200.dat"};
    *filesSize = sizeof values / sizeof *values;

    *files = NULL; // crucial for first realloc

    for (int i = 0; i < *filesSize; i  ) {
        *files = realloc(*files, (i 1) * sizeof **files);
        // I'm using strlen here instead of MAX_FILE_NAME_LENGTH but
        // you can change that back if you want more space than the
        // strings really need.
        (*files)[i] = malloc((strlen(values[i])   1) * sizeof(char));
        strcpy((*files)[i], values[i]);
    }
}

int main(int argc, char *argv[]) {
    char **files;
    size_t filesSize = -1;

    loadList(&files, &filesSize);
    // printf("filesize= %d\n",filesSize );
    for (int i = 0; i < filesSize; i  ) printf("%s\n", files[i]);

    // free what was allocated in the loop
    for (int i = 0; i < filesSize; i  ) free(files[i]);
    free(files);

    return 0;
}

One note on files = realloc(*files, ... though: If realloc fails you've lost the original pointer and will not be able to recover. Either assign the return value to a temporary variable and check that it worked or return from the function (after setting *filesSize to i) or exit the program if *files == NULL.

CodePudding user response：

The rule is that a free must exist for each xalloc call. And using a return value can avoid an additional indirection level.

A first way would be to return an array of pointers if you want to only allocate the used size for each string:

char** loadList(int* filesSize);

int main(int argc, char* argv[])
{
    char** files;
    int filesSize = -1;

    files = loadList(&filesSize);
    //printf("filesize= %d\n",filesSize );
    for (int i = 0; i < filesSize; i  ) printf("%s\n", files[i]);
    for (int i = 0; i < filesSize; i  ) free(files[i]);
    free(files);

    return 0;
}

////////////////////////////////////////////////////////////////////////////////

char ** loadList(int* filesSize) {
    //some arry for the example
    char* values[] = { "100.dat", "150.dat", "200.dat" };
    *filesSize = sizeof(values) / sizeof(values[0]);
    char** files = malloc(*filesSize * sizeof(*files));
    for (int i = 0; i < *filesSize; i  ) {
        files[i] = strdup(values[i]);
        printf("%s\n", files[i]);
    }
    return files;
}

Alternatively if you want all file name to have the maximum size, it is probably better to build a 2D array to have one single allocation/deallocation step:

#define MAX_FILE_NAME_LENGTH 255

typedef char FileName[MAX_FILE_NAME_LENGTH];

FileName* loadList(int* filesSize);

int main(int argc, char* argv[])
{
    int filesSize = -1;

    FileName *files = loadList(&filesSize);
    //printf("filesize= %d\n",filesSize );
    for (int i = 0; i < filesSize; i  ) printf("%s\n", files[i]);
    free(files);

    return 0;
}

////////////////////////////////////////////////////////////////////////////////

FileName* loadList(int* filesSize) {
//some arry for the example
    char* values[] = { "100.dat", "150.dat", "200.dat" };
    *filesSize = sizeof(values) / sizeof(values[0]);
    FileName *files = malloc(*filesSize * sizeof(*files));
    for (int i = 0; i < *filesSize; i  ) {
        strcpy(files[i],values[i]);
        printf("%s\n", files[i]);
    }
    return files;
}