How can I change an array of string pointers in another function?

Question

I have been trying to figure out how to modify an array of char pointers but no matter what I do there appears to be no change below are the three arrays I'm trying to change including the call to the function I'm using.

char*cm1[5];
char*cm2[5];
char*cm3[5];
setupCommands(&cm1,commands,file,0);
setupCommands(&cm2,commands,file,1);
setupCommands(&cm3,commands,file,2);

The code below is the function itself.I was thinking that maybe it involves a double pointer but if I try *cmd to change the array I get a segmentation fault.

void setupCommands(char **cmd[], char* commands[],char file[],int index){



    char str1[255];
    strcpy(str1,commands[index]);
    char newString [5][255];
    int j = 0;
    int ctr = 0;
    int i;
    //printf("str1 %s\n" ,str1);

    for(i = 0; i <= strlen(str1); i++){

        if(str1[i] == ' '|| str1[i] =='\0'){
            newString[ctr][j] = '\0';
            ctr++;//next row
            j=0;// for next word, init index to 0
        }else{
            newString[ctr][j]=str1[i];
            j++;
        }
    }


    for(i = 0; i < ctr; i++){
            //printf(" test2 %s \n", newString[i]);
        cmd[i] = newString[i];
            //printf(" test2 %d %s \n", i,cmd[i]);
    }
 //printf("index %d", i);
  cmd[i]= file;
  cmd[i + 1] = NULL;

  //execvp(cmd[0],cmd);
   //cmd
}

An obvious problem here: You only create objects with automatic storage duration (the default for local variables) in your function. Once the function exits, these objects don't exist any more. — user2371524
– user2371524, Commented Apr 30, 2018 at 7:13
With a function interface like this, the implementation has to use malloc() and friends (and the caller would be responsible for cleanup with free()) — user2371524
– user2371524, Commented Apr 30, 2018 at 7:14
cmd[i] = newString[i]; consider what newString[i] is pointing at. This is a shallow copy. — cdarke
– cdarke, Commented Apr 30, 2018 at 7:18
the variable ctr must not exceed 4, but there is nothing in the code to enforced that criteria — user3629249
– user3629249, Commented Apr 30, 2018 at 18:47

малин чекуров · Accepted Answer · 2018-05-03 06:47:02Z

There are a few issues with your code:
- you are trying to return references to the local 'char newString [5][255]' when the function exits. In simple worlds - never return anything locally allocated on the stack. This is the reason you are getting the segmentation fault.
- char **cmd[] must be declared char *cmd[] - even though you will get a warning from the compiler assignment from incompatible pointer type, the code would run and execute correctly(essentially **cmd[] would do the same work as *cmd[], even though it's not of correct type) if you didn't return references to the local object;

Easy and simple optimization is just to remove the array str1 and directly operate on the array commands.

Apart from this simple optimization I have changed your code to overcome the segmentation fault, by allocating on the heap, instead on stack(will live until the program terminates) the multidimensional array, and I also calculate it's size so I will know how much memory to allocate. Now it's safe to return references to it.

Note that more optimizations could be made, but for the sake of the simplicity this is the bare minimal for this code to work.

int setupCommands(char *cmd[], char *commands[], char file[], int index)
{
int j = 0;
int ctr = 0;
int i = 0;

int rows = 0;
int cols = 0;

char **newString = NULL;

while(commands[index][i])
{
    if (commands[index][i] == ' ')
    {
        ++rows;
    }

    ++i;
}
++rows;

cols = strlen(commands[index]) + 1;

newString = malloc(rows * sizeof(*newString));
if (newString == NULL)
{
    return -1;
}

for (i = 0; i < rows; ++i)
{
    newString[i] = malloc(cols * sizeof(*newString));
    if (newString[i] == NULL)
    {
        return -1;
    }
}

for(i = 0; i <= strlen(commands[index]); i++){

    if(commands[index][i] == ' '|| commands[index][i] =='\0'){
        newString[ctr][j] = '\0';
        ctr++;//next row
        j=0;// for next word, init index to 0
    }else{
        newString[ctr][j]=commands[index][i];
        j++;
    }
}

for(i = 0; i < ctr; i++){

    cmd[i] = newString[i];

}

cmd[i]= file;
cmd[i + 1] = NULL;

return 0;
}

0___________ · Accepted Answer · 2018-04-30 08:21:01Z

2

First of all - being the three stars pointer programmer is not good :)

You assign it with pointer to the local variable which is not longer available after the function return

But if you still want the three stars pointers:

char **cm1;
char **cm2;
char **cm3;
setupCommands(&cm1,commands,file,0);
setupCommands(&cm2,commands,file,1);
setupCommands(&cm3,commands,file,2);

#define MAXWORD 256

int setupCommands(char ***cmd, const char *commands,const char *file,int index){

    char str1[255];
    strcpy(str1,commands[index]);

    int j = 0;
    int ctr = 0;
    int i;
    //printf("str1 %s\n" ,str1);

    *cmd = malloc(sizeof(char *));
    **cmd = malloc(MAXWORD);
    if(!*cmd || !**cmd) 
    {
        /* do spmething if mallocs failed*/
        return -1;
    }
    for(i = 0; i <= strlen(str1); i++){

        if(str1[i] == ' '|| str1[i] =='\0'){
            (*cmd)[ctr][j] = '\0';
            ctr++;//next row
            *cmd = realloc((ctr + 1) * sizeof(int));
            (*cmd)[ctr] = malloc(MAXWORD);
            if(!*cmd || !*cmd[ctr]) 
            {
            /* do spmething if mallocs failed*/
                return -1;
            }

            j=0;// for next word, init index to 0
        }else{
            (*cmd)[ctr][j]=str1[i];
            j++;
        }
    }


    *cmd = realloc(sizeof(char *)  * ctr + 2)  
    (*cmd)[ctr - 2] = malloc(MAX);
    if(!*cmd || !*cmd[ctr - 2]) 
    {
        /* do spmething if mallocs failed*/
        return -1;
    }
    strcpy((*cmd)[ctr - 2], file);
    (*cmd)[ctr - 1] = NULL;

    return 0;

  //execvp(cmd[0],cmd);
   //cmd
}

you can improve many things (for example do not realloc every time but in the larger chunks) and I did not change anything in your code logic.

edited Apr 30, 2018 at 8:21

answered Apr 30, 2018 at 7:14

0___________

71.6k4 gold badges41 silver badges94 bronze badges

2 Comments

user2371524 Over a year ago

You really think this is enough for an answer here?

cdarke Over a year ago

For an answer you should at least show which code is in error and how to correct it.

Collectives™ on Stack Overflow

How can I change an array of string pointers in another function?

2 Answers 2

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related