Take user input and then do execvp

Question

So I have the following C code which asks the user to give a command (valid in unix), and then I have to take the string and split it in an array in order to execute the command that the user gave with execvp(). It compiles but the execvp doesn't seem to work. Is something wrong in the way I split the user's input in an array? PS: Some of the includes aren't neccessary but it's not the final program.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <signal.h>

main() {

  char str[64];
  int i =0;
  char *p = strtok(str," ");

  printf("Please give me a Unix command! :\n");
  gets(str);
  char  *array[sizeof(str)];
  while (p!=NULL) {
    array[i++] = p;
    p = strtok (NULL, " ");
  }

execvp(str ,array);
perror("execvp");
}

The output I get when I run this is:

Please give me a Unix command! :
ls -l
execvp: No such file or directory

[DO NOT use `gets()`, it is dangerous](http://stackoverflow.com/q/1694036/2173917). use [`fgets()`](https://linux.die.net/man/3/fgets) instead. — Sourav Ghosh, Jan 13 '17 at 16:30

score 1 · Accepted Answer · answered Jan 13 '17 at 16:53

You're calling strtok(str, " ") before str has any information.

Simply call it after you get input:

main() {
  char str[64];
  char *array[sizeof(str)];
  char *p = NULL;
  int i = 0;

  printf("Please give me a Unix command! :\n");
  fgets(str, sizeof(str), stdin);  // Use fgets instead of gets.

  p = strtok(str," ");

  while (p != NULL) {
    array[i++] = p;
    p = strtok(NULL, " ");
  }

  execvp(str, array);
}

score 0 · Answer 2 · edited May 23 '17 at 10:29

The first problem as I see here is with

char *p = strtok(str," ");

as you're trying to read indeterminate values. str is not initialized, there's no guarantee that there is a null-terminator present there which makes this a string. So, you're essentially invoking undefined behavior.

That said,

gets(), it is dangerous as it lead to buffer overflow. use fgets() instead.
for an array variable, sizeof(str) does not give you the size of the content, it returns you the size of the whole array. You might want to use strlen() to get the length of the string, but remember, strlen() does not count the null-terminator.

Take user input and then do execvp

2 Answers2