1

My program has a variable number of args and I need to make an execv with a new path, so I want to change the value of argv[1] in a different variable without changing it, but it won't let me.

char** arg_exec = malloc(argc * sizeof (char*));
int i;
for(i=0;i <= argc-1; i++)
   arg_exec[i] = strdup(argv[i]);
arg_exec[argc] = NULL;
if( (pid = fork()) == 0){
    arg_exec[1] = strcat(directory , dir_info->d_name); //some variables with the current path and a name
    execv(arg_exec[0], arg_exec);
    printf("Error in process %d\n", getpid());
    return 1;
 }

but after it runs this line arg_exec[1] = strcat(directory , dir_info->d_name); it changes my value of argv[1], and my program fails..

It worked fine with execl, since it was like execl(argv[0],strcat(directory , dir_info->d_name), ..., NULL); but because I have a variable number of arguments to run it, it wouldn't be good to implement that way.

Edit1: Added NULL at the end of the array Edit2: I'm doing a version of the find, so strcat will add to the current directory a folder to look into. This is the initalization of the directory: char *directory = strcat(argv[1],"/");

J. Seixas
  • 57
  • 1
  • 1
  • 9

3 Answers3

2

The code as shown misses to NULL-terminate the target pointer array arg_exec.

To do so allocate one more element then needed and explicitly set it to NULL.


Also the second assignment the code does to arg_exe's elements overwrites the result of the first, causing a memory leak, as the address to the memory allocate by strdup() is lost.


Assuming argv gets passed in via main(), then this line

char *directory = strcat(argv[1],"/");

tries to concatenate "/" to what argv[1] is pointing to and with this writes beyond the bounds of argv[1] and by doing so invokes undefined behaviour. From then on anything can happen, from crash to seeming to work.

BTW; please note that directory is just a pointer to char, so it does not provide any memory to store any "string"-like values.

What you probably want instead of

char *directory = strcat(argv[1],"/");

...

  arg_exec[1] = strcat(directory , dir_info->d_name); 

is something like this (this assumes argv[1] holds any "base"-directorie's name and argv_exe is allocated and initialised as per your (corrected) code):

{
  void * tmp = realloc(arg_exec[1], 
    strlen(arg_exec[1]) + strlen("/") + strlen(dir_info->d_name) + 1);
  if (NULL == tmp)
  {
    perror("realloc() failed");
    exit(1);
  }

  arg_exec[1] = tmp;
}

strcat(arg_exec[1], "/");
strcat(arg_exec[1], dir_info->d_name);
alk
  • 69,737
  • 10
  • 105
  • 255
  • I don't know why, but i was always getting an error with realloc. Another answer solved my problem, anyway appreciated – J. Seixas Apr 07 '17 at 21:34
2

You do not show how directory is set. I suspect that it's something like

char *directory = argv[1];

In that case, strcat will modify the location directory is pointing to and hence argv.

2

char *directory = strcat(argv[1],"/"); attemps to modify argv[1] beyond its allocation, which is UB. @alk.

Modifying argv itself may be UB. Is argv[n] writable?

So allocate memory for both.

Note: char** arg_exec = malloc(argc * sizeof (char*)); is insufficient as argv[argc] must be NULL. Need 1 more. Notice argc is not passed to execv().


Step 1. Make a copy of the pointer array argv[]

char **argv_new;
size_t a_size = sizeof *argv_new * (argc + 1);  // + 1 for the final NULL
argv_new = malloc(a_size);
memcpy(argv_new, argv, a_size);

Step 2. Form the new arg[1]

int size = 1 + snprintf(NULL, 0, "%s/%s", argv[1], dir_info->d_name);
argv_new[1] = malloc(size);
snprintf(argv_new[1], size, "%s/%s", argv[1], dir_info->d_name);

Use it

execv(arg_new[0], arg_new);
free(argv_new[1]);
free(argv_new);

TBD: Error checking to add for: argc > 1, malloc(), snprintf(), execv()

Community
  • 1
  • 1
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256