warning: passing arg 2 of `execv' from incompatible pointer type

Hello. I am trying to write a shell program, one that takes in 2 UNIX commands and executes it. the format for input is:
<command> | <command>
My problem is that for some reason, the execv call is erroring out each time. Relevant parts of my code are below. I removed the parts for the second command, as well as creating a pipe since I know those are working correctly. I know my problem is with calling execv.

Code:

---

#include <unistd.h>
#include <stdio.h>
#define WRITE 1
#define READ 0
#define BUFFSIZE 200
#define FILENAME "Pipe"
#define CHECK(FUNCTION, STATUS) if(STATUS < 0) \
{\
        printf("\nfailed to %s %s. code returned was: %d\n", FUNCTION, FILENAME, STATUS); \
        return 0; \
}


int main(int argc, char** argv) {
        int check;
        char buffer[BUFFSIZE];
        do {
       
                printf("shell-prompt>> ");
                gets(buffer);
                if(buffer[0] == '@') {
                        break;
                }
               
                char firstargs[10][BUFFSIZE];
                char secondargs[10][BUFFSIZE];
             
                firstargs[0][0] = '/';  //initializing first argument so that it executes from /bin
                firstargs[0][1] = 'b';
                firstargs[0][2] = 'i';
                firstargs[0][3] = 'n';
                firstargs[0][4] = '/';
               
                //getting part 1
                int i = 0, countr = 0, countc = 5;
               
                while(*(buffer+i) == ' ' || *(buffer+i) == '\t') { //while we see leading whitespace
                        i++;  //skip it
                }  //end whitespace loop
                while(*(buffer+i) != '|' && *(buffer+i) != '\0') { //while we have not see a bar or end of string (sanity check)
                        //check for whitespace
                        int whitecount = 0;
                        while(*(buffer+i) == ' ' || *(buffer+i) == '\t') { //while we see whitespace
                                i++;  //skip it
                                if(whitecount == 0) {  //if this was the first whitespace seen
                                       
                                        firstargs[countr][countc] = NULL; //null terminate old arg
                                       
                                        countr++;  //increment argc number
                                        whitecount = 1;  //no longer see first whitecount
                                        countc = 0;  //reset position on new argument
                                }
                        }  //end whitespace loop
                        //after whitespace
                        if(*(buffer+i) == '|') {
                                break;
                        }
                       
                        firstargs[countr][countc] = buffer[i];  //copy character
               
                        countc++;  //increment position in arg
                        i++;  //increment position in buffer
                }
               
                firstargs[countr][countc] = NULL; //null terminate old arg
               
                firstargs[countr+1][0] = NULL; //null terminate last string
       
                /* now we have arguments for first and second process*/
                //DEBUG PRINTING
                int printtemp = 0;
                while(firstargs[printtemp][0] != 0) {
                        printf("firstargs[%d]: %s\n", printtemp, firstargs[printtemp]);
                        printtemp++;
                }
                int childA, childB;
                childA = fork();
                CHECK("fork", childA)
                if(childA == 0) {
                        printf("I am process A\n");
                        check = execv(firstargs[0], firstargs);
                        CHECK("execv", check);
                        exit(0);
                }

                int childreturn;
                //waits for 2 children to finish
                check = wait(&childreturn);
                CHECK("wait", check);
                free(firstargs);

        } while(1);
        exit(0);
}

---

Judging by the warning message gcc gives me, I think the problem is with the second argument of execv, but I honestly have no clue what it doesn't like, since it is a char[][].

Any help or nudges in the right direction would be greatly appreciated. Thanks!

The problem is that execv() wants an array of char*, not an array of array. Although it's often taught that arrays and pointers are equivalent, they are not. You'll need to make an array of char*, which means you'll probably need to use malloc() (or strdup() since you're on a unix-like platform) to allocate space.

You also have some other issues:

Code:

---

firstargs[countr][countc] = NULL;

---

NULL is a null pointer constant, whereas firstargs[countr][countc] is a char, not a pointer. To null terminate a string, use 0, not NULL. You're doing this a few places in your code.

You want to #include <stdlib.h> because you're using exit(), and sys/wait.h because you're using wait().

Calling free(firstargs) is wrong because you didn't allocate it. You should only call free() on something that you've allocated with malloc(), calloc(), or realloc() (or some other platform-specific allocation function, like strdup()). You should never free a local variable.

You appear to be using gcc. Always build with the -Wall option. This enables extra warnings that you should heed.

SourceForge.net: Gets - cpwiki

Quote:

---

The problem is that execv() wants an array of char*, not an array of array. Although it's often taught that arrays and pointers are equivalent, they are not. You'll need to make an array of char*, which means you'll probably need to use malloc() (or strdup() since you're on a unix-like platform) to allocate space.

---

Yeah, I was taught that they are the same thing. So, here is some updated code:

Code:

---

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/wait.h>
#define WRITE 1
#define READ 0
#define BUFFSIZE 200
#define FILENAME "Pipe"
#define CHECK(FUNCTION, STATUS) if(STATUS < 0) \
{\
        printf("\nfailed to %s %s. code returned was: %d\n", FUNCTION, FILENAME, STATUS); \
        return 0; \
}


int main(int argc, char** argv) {
        int check;
        char buffer[BUFFSIZE];
        do {
       
                printf("shell-prompt>> ");
                fgets(buffer, sizeof(buffer), stdin);
                if(buffer[0] == '@') {
                        break;
                }
               
                char *firstargs[10];
                char *secondargs[10];
               
                int j;
                for(j = 0; j< 10;j++) {
                        firstargs[j] = malloc(BUFFSIZE*sizeof(char));
                        secondargs[j] = malloc(BUFFSIZE*sizeof(char));
                }
               
                *(firstargs[0]) = '/';
                *(firstargs[0]+1) = 'b';
                *(firstargs[0]+2) = 'i';
                *(firstargs[0]+3) = 'n';
                *(firstargs[0]+4) = '/';
               
                //getting part 1
                int i = 0, countr = 0, countc = 5;
               
                while(*(buffer+i) == ' ' || *(buffer+i) == '\t') { //while we see leading whitespace
                        i++;  //skip it
                }  //end whitespace loop
                while(*(buffer+i) != '|' && *(buffer+i) != '\0') { //while we have not see a bar or end of string (sanity check)
                        //check for whitespace
                        int whitecount = 0;
                        while(*(buffer+i) == ' ' || *(buffer+i) == '\t') { //while we see whitespace
                                i++;  //skip it
                                if(whitecount == 0) {  //if this was the first whitespace seen
                                       
                                        *(firstargs[countr]+countc) = 0; //null terminate old arg
                                        countr++;  //increment argc number
                                        whitecount = 1;  //no longer see first whitecount
                                        countc = 0;
                                }
                        }  //end whitespace loop
                        //after whitespace
                        if(*(buffer+i) == '|') {
                                break;
                        }
                       
                        *(firstargs[countr]+countc)= buffer[i];  //copy character
               
                        countc++;  //increment position in arg
                        i++;  //increment position in buffer
                }
               
                *(firstargs[countr]+countc) = 0; //null terminate old arg
                countc++;
                *(firstargs[countr]+countc) = 0; //null terminate last string
               
                i++;  //incrementing to move away from the bar
               
                /* now we have arguments for first and second process*/
                int childA;
                childA = fork();
                CHECK("fork", childA)
                if(childA == 0) {
                        printf("I am process A\n");
                        printf("my name is %s", firstargs[0]);
                        check = execv(firstargs[0], firstargs);
                        CHECK("execv", check);
                        exit(0);
                }
               
                int childreturn;
                //waits for 2 children to finish
                check = wait(&childreturn);
                CHECK("wait", check);
                for(j = 0; j < 10; j++) {
                        free(firstargs[j]);
                        free(secondargs[j]);
                }
        } while(1);
        exit(0);
}

---

However, even with fixing the char[][] to a *char[] , execv is still erroring out, even though I get no more compile time warnings (thanks for the -Wall tip, and that nice tip for fgets). I added a debug statement in the child that has is print out the name of the command it is trying to execute, and for a test case of:
more test.c
it returns:
"my name is /bin/more"
which I confirmed to be where "more" is on my system.

Thank you so much for your help.

The argument list to execv() must be NULL terminated--you need a NULL pointer right after your last argument. Something like firstargs[countr + 1] = NULL, before you execv(), should work.

Also, fgets(), unlike gets(), stores the newline it reads. You'll want to find it (with strchr()) and remove it, if it exists.

Quote:

---

The argument list to execv() must be NULL terminated--you need a NULL pointer right after your last argument. Something like firstargs[countr + 1] = NULL, before you execv(), should work.

Also, fgets(), unlike gets(), stores the newline it reads. You'll want to find it (with strchr()) and remove it, if it exists.

---

Thank you so much for your help. My code is now working ( I had a few logic bugs to work out on my own). I do have some general language questions from this if you (or someone else from here) wouldn't mind answering.

What exactly is the difference between *char and char[]? from my understanding when you declare
char c[10];
then c is a pointer to the first element in the array, and can be accessed like a pointer *c.

By extension: how come when creating int main() you can write:

Code:

---

int main(int argc, char* argv[])

---

and

Code:

---

int main(int argc char** argv)

---

and access argv in the program in the same way, as in

Code:

---

char array[] = argv[1];

---

even though they are different data structures?

Thanks again.
-luro

char c[10] creates an array of 10 elements. "c" is an array. Not a pointer.
What is true, however, is that when you refer to the array's name, it decays to a pointer. That makes people believe it is a pointer, but that is untrue.

Regarding
int main(int argc, char* argv[])
and
int main(int argc char** argv)

When used as parameters, array notation such as char* argv[] is simply a pointer. Yes, it's silly and stupid, but that's how it is. Therefore, those two lines are equivalent.

char array[] = argv[1];
would take argv[1] and copy it into array, since argv[n] is a pointer, and not an array.