Thread: 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.

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.

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