I am reading "Operating System: Three Easy Pieces". In chapter 5, there is a piece of code that show the usage of exec() system call.

    1 #include "common.h"
    2
    3 int main(int argc, char ** argv) {
    4     printf("hello world (pid: %d)\n", (int) getpid());
    5     int rc = fork();
    6     if (rc < 0) {
    7         fprintf(stderr, "fork failed\n");
    8         exit(1);
    9     } else if (rc == 0) {
   10         printf("hello, I am child (pid: %d)\n", (int) getpid());
   11         char *myargs[3];
   12         myargs[0] = strdup("wc");
   13         myargs[1] = strdup("p3.c");
   14         myargs[2] = NULL;
   15         execvp(myargs[0], myargs); // run work count
   16         printf("this shouldn't print out\n");
   17     } else {
   18         int wc = wait(NULL);
   19         printf("hello, I am parent of %d (wc: %d) (pid: %d)\n", rc, wc, (int) getpid());
   20     }
   21     return 0;
   22 }
   23

Why is strdup() used in line 12~14 ? I've tried to replace this part with the following:

   12         myargs[0] = "wc";                                                                                                                                                                                     
   13         myargs[1] = "p3.c";
   14         myargs[2] = NULL;

It works just as the previous one.

CodePudding user response：

The code compiles with your change, but it might trigger undefined behaviour.

execvp expects a parameter char *const argv[]. Not the missing const on the char. So execvp is free to write to the strings passed to it.

String literals in C are not modifiable:

String literals are not modifiable (and in fact may be placed in read-only memory such as .rodata). If a program attempts to modify the static array formed by a string literal, the behavior is undefined.

So the author of the code wants to be on the safe side and strdups the string literals to get a modifiable version of the string.

Please note that C is more strict and there gcc would emit

warning: ISO C forbids converting a string constant to 'char*' [-Wwrite-strings]

for your code.

If execvp does not modify the arg strings, then it would be better if it had a signature that expressed this (const char *const argv[]).

CodePudding user response：

Your version of the code is fine. Contrary to Werner's answer, it will not cause undefined behavior.

According to the POSIX spec, the function prototype for execvp is

int execvp(const char *file, char *const argv[]);

Taken all by itself, this would suggest that the contents of the strings in the argv array could be modified, although the pointer elements of that array themselves could not. If that were so then it would indeed be necessary to pass an array of pointers to writable strings, such as strdup would provide, and not string literals, since writing to a string literal is indeed undefined behavior. This may have been what the author of the code was thinking.

However, the text of the spec gives us more information:

The argv[] and envp[] arrays of pointers and the strings to which those arrays point shall not be modified by a call to one of the exec functions, except as a consequence of replacing the process image.

So in fact execvp is guaranteed not to modify those strings. As such it is perfectly safe to pass a string literal. The author of the original code might not have known about this guarantee.

The rationale further down explains why they chose the prototype as they did ("The statement about argv[] and envp[] being constants is included to..."). As Werner suggests, const char *const argv[] would seem to better express the actual behavior. However, C's pointer conversion rules mean that if they had, then code like

char *args[5];
args[0] = /* some writable string */;
...
execvp(file, args);

would not compile, even though in principle it is perfectly fine. The rationale includes a politely veiled rant about the semantics of const in C and how they make it impossible to write this properly.

There is some further discussion at Can I pass a const char* array to execv?.