Thread: Something wrong with execv?

Hi

I am trying to get a line from the user and execute the command.Here are sme parts of my program

Code:

/* A structure for command and parameters */
typedef struct Command_t {
	char *name;
	int argc;
	char *argv[];
} Command;
......
......
/* Parses the command line and stores the result into a Command structure */
/* This function is taken from the Gary Nutt's Operating Systems book */
int parseCommand(char *cLine, Command *cmd)
{
	int argc;
	char **clPtr;
	/* Initialization */
	clPtr=&cLine; /* cLine is the command line */
	argc = 0;
	cmd->argv[argc]=(char *)malloc(MAX_ARG_LEN);
	/* Fill argv */
	while ((cmd->argv[argc]=strsep(clPtr,WHITESPACE)) != NULL)
	{
		cmd->argv[++argc]=(char *) malloc(MAX_ARG_LEN);
	}
	
	//cmd->argv[argc]=(char *)NULL;
	
	/* Set the command name and argc */
	cmd->argc= argc-1;
	cmd->name= (char *)malloc(sizeof(cmd->argv[0]));
	strcpy(cmd->name,cmd->argv[0]);
	
	return 1;
}
......
......
/*Runs a command */
int runCommand(char *command_str,Command cmd)
{
	int status;
	pid_t pid;

	pid=fork();
	if (pid<0)
	{
		fprintf(stderr,"Cannot create child process!\n");
		exit(1);
	}
	else if (pid==0)
	{
		if (execv(command_str,cmd.argv)==-1)
			fprintf(stderr,"Cannot execute command!\n");
	}
	else 
	while (wait(&status) != pid);
		
	return status;
}
....
....

Well. When I entered a command like "ls -l",my program just excutes /bin/ls not with the paramters.

What is wrong?

> char *argv[];
Well this isn't valid, you need to do char **argv and allocate an array of char pointers.
Or just have char *argv[100]; and limit the number of arguments.

> cmd->argv[argc]=(char *)malloc(MAX_ARG_LEN);
You also need to follow up with a strcpy() of the relevant string as well.

> cmd->argv[argc]=(char *)NULL;
Yes, you need this as well.

> int runCommand(char *command_str,Command cmd)
Your're passing cmd "by value", which means all the pointers you malloc'ed are now duplicated, but only as a shallow copy.

> execv(command_str,cmd.argv)
If you created argv[] properly, then you should be able to do
execv(cmd.argv[0],cmd.argv)

Hi

Thaks for advices but

When I change char *argv[] to char **argv I get "Segmantation Fault" while running the command.

Passing a pointer to cmd to runCommand() makes the program bevahes as if there is a paramter after each command. For example,

Code:

fnoyan@helix:~/c/launch> ./msh
fnoyan % ls
ls: : No such file or directory
fnoyan %

So, it forks and runs the command. But passes an extra paramter to the program!

I use command_str as the full path+comman name, for example command_str for ls is "/bin/ls". I have another function that finds the exact path of the command. Othervise, the program displays a "Command not found" message.

I used printf() to check the passed parameters to the function (I mean, printf(..,cmd.argv[1],...)), it seems that all the parameters are passed successfully! Here is the new code fragment from runCommand()

Code:

	else if (pid==0)
	{
		printf("%s\n%s\n%s\n",command_str,cmd.name,cmd.argv[1]);
		if (execv(command_str,cmd.argv)==-1)
			fprintf(stderr,"Cannot execute command!\n");
	}
	else

and again it gives the same output as I pasted above (for the ls command)!

I don't want to change the value of cmd, so, do I have to pass a pointer?

> When I change char *argv[] to char **argv I get "Segmantation Fault" while running the command.
Well did you allocate it?

Like
cmd->argv = malloc ( 100 * sizeof *cmd->argv );

Well, I malloc() and the same thing occurs! It makes me crazy! I

Code:

#define MAX_PARAM_LEN blabla
...
...
cmd->argv = malloc ( MAX_PARAM_LEN * sizeof *cmd->argv );

I added the malloc in the parseCommand() function

Code:

	argc = 0;
	cmd->argv = malloc ( MAX_PARAM_LEN * sizeof *cmd->argv );
	cmd->argv[argc]=(char *)malloc(MAX_ARG_LEN);

The program always passes an extra argument to execv!

How about pasting the whole thing then?
Something that anyone can compile and run, and see what is going on.

I was thinking of something like this

Code:

int parseCommand(char *cLine, Command *cmd)
{
  int     argc = 0;
  char  **clPtr = &cLine; /* line to be tokenised (and destroyed) */
  char   *p = NULL;

  cmd->argv = malloc ( MAX_PARAM_LEN * sizeof *cmd->argv );

  /* Fill argv */
  while ( (p=strsep(clPtr,WHITESPACE)) != NULL )
  {
    cmd->argv[argc]=malloc( strlen(p) + 1 );
    strcpy( cmd->argv[argc], p );
    argc++;
  }

  cmd->argv[argc]=NULL;

  /* Set the command name and argc */
  cmd->argc= argc-1;
  cmd->name = malloc( strlen(cmd->argv[0])+1 );
  strcpy( cmd->name, cmd->argv[0] );

  return 1;
}