how to store command line arguments into a dynamic string array in c-CodePudding

Hi I'm trying to store command line arguments of unknown amount into a dynamically allocated array of strings in c. The terminal is telling me I have a segFault which I have tracked down to a strcpy line(see code 1.) I've tried looking at other peoples solutions but I'm still not sure what I have done wrong, but I believe I have a memory leak.

code 1.

for (int j = fileLocCount 1; j < argc; j  ){
    strcpy(filelist.array[filelist.used], argv[j]);
    filelist.used  = 1;
    if (filelist.used == filelist.size){
        for (int i = 0; i < 100; i  )
          insertArray(&filelist, i);
    }
}
//printArray(&filelist);
freeArray(&filelist);

the 'filelist' variable is a struct called Array

typedef struct {
  char **array;
  size_t used;
  size_t size;
} Array;

and has the functions

void initArray(Array *a, size_t initialSize) {
  a->array = (char **) calloc(initialSize, 255);
  a->used = 0;
  a->size = initialSize;
}
void insertArray(Array *a, int element) {
  // a->used is the number of used entries, because a->array[a->used  ] updates a->used only *after* the array has been accessed.
  // Therefore a->used can go up to a->size
  if (a->used == a->size) {
    a->size *= 2;
    a->array = realloc(a->array, a->size * sizeof(int));
  }
    a->array[a->used  ] = a->array[element];
}
void freeArray(Array *a) {
  free(a->array);
  a->array = NULL;
  a->used = a->size = 0;
}

Any help is really really appreciated

CodePudding user response：

You have contradicting (and both wrong, I think) ?alloc:

  a->array = (char **) calloc(initialSize, 255);

    a->array = realloc(a->array, a->size * sizeof(int));

a->array is a char **, so a pointer to a string (which itself is a pointer to a char), implementing an array of strings, which, each of them, are an array of chars.

So I don't see why you need 255 bytes per pointers.

Each pointer (to a char) in your array of pointers to char, should be the size of a pointer. So you need initialSize times the size of a pointer here

a->array = (char **) calloc(initialSize, sizeof(char *));

And then, you need to alloc, each string itself (for now you allocated just pointers to them)

for(int i=0; i<initialSize; i  ) a->array[i]=malloc(255*sizeof(char));

(the sizeof(char) is here more for clarity of the code, since it is 1. But well, you never know, one day your code may have to compile on a computer with 2 bytes char)

So that first calloc should be

a->array = (char **) calloc(initialSize, sizeof(char *));
for(int i=0; i<initialSize; i  ) a->array[i]=malloc(255*sizeof(char));

And for the realloc, it has the same problem. Although, this times, instead of wrongly allocating 255 bytes per pointer, you allocated sizeof(int) bytes per pointers.

So, the realloc should be

a->array = realloc(a->array, a->size * sizeof(char *));
for(int i=a->size/2; i<a->size; i  ) a->array[i]=calloc(255, sizeof(char));

In both case, your main problem was the missing allocation for the strings themselves. In the first case, you allocated memory for the pointers. With a wrong size (you allocated 255 bytes per pointer, when 8 or 4 are usually enough, that is sizeof(char *)). So, you probably add more than enough memory to store your pointers, just a waste of memory. But then, you add no memory to store the chars themselves in each of those strings.

In the second case, same thing, but the allocation of memory for pointers wrong size was different. You allocated sizeof(int) bytes per pointer, when you should have allocated sizeof(char *). And there, on most machine, it would be either, by coincidence, the same thing (4 bytes), or it could be not enough (4 bytes instead of 8).

Note that

I surmise that you meant to allocate 255 chars per string (else it is a mystery why this 255)
I did no allocation test (one should check if *alloc returned null before proceeding).
I did not really check the rest of the code, since that seemed to be a major segfault source to begin with.

CodePudding user response：

Your memory allocation is wrong.

For something like this where there is no fixed limit for the length of the individual strings, you need what is known as a "jagged array".

See https://en.wikipedia.org/wiki/Jagged_array#:~:text=In computer science, a jagged,edges when visualized as output.

It is an array of char-pointers where each char-pointer points to an array of char where the actual strings are stored.

So there are two allocation steps:

Allocate the array of char-pointers
Allocate the char-array for each string to be stored

Step 1 will look like:

#define INITIAL_NUMBER_OF_POINTERS 8
char** array = calloc(INITIAL_NUMBER_OF_POINTERS, sizeof *array);
if (array == NULL) exit(1);

Step 2 will (for each string you want to copy) look like:

size_t sz = strlen(string_to_copy)   1;
array[i] = malloc(sz);
if (array[i] == NULL) exit(1);
strcpy(array[i], string_to_copy);

(note: If you are willing to use the non-standard strdup function step2 can be written much easier)

You can afterwards use realloc on array to be able to store additional char-pointers, i.e. prepare for more string copies.