C segmentation fault after altering argv

Question

I want to change the values of argv in C, but I'm getting a segmentation fault. Here's the code.

#include <stdio.h>
#include <string.h>

int main(int argc, char **argv)
{
    for (int i = 1; argv[i]; i++)
    {
        char *val;

        printf("before: %d %s\n", i, argv[i]);
        argv[i] = "bar=foo";
        printf("after: %d %s\n", i, argv[i]);

        char *arg = argv[i];
        val = strchr(arg, '=');
        *val = '\0';
    }
    return 0;
}

I'm passing the argument foo=bar (and try to change it in line 11 to bar=foo). The output looks like this:

before: 1 foo=bar
after: 1 bar=foo

So, the modification actually takes place, but the line *val = '\0'; causes a segmentation fault.

Can somebody tell me why this is and how I can prevent it?

if you are passing only 1 parameter then argv[i] is only valid if i takes on value 0. in your for loop the first iteration you are checking for argv[1] as a predicate to see if you should keep looping, but this will not work. — ForeverStudent, Dec 17 '15 at 14:50
@IIIIIIIIIIIIIIIIIIIIIIII `argv[0]` is always the name of the program, if you pass one arg, `argv[1]` is valid and `argv[2]` should be [`NULL`](http://stackoverflow.com/a/11020198/2666289). — Holt, Dec 17 '15 at 14:54
@Holt: 1) `NULL` is a macro with a _null pointer constant_. A pointer cannot be a macro. It can be a _null pointer_, however, which equals a _null pointer constant_. 2) Please point me at where the standard says `argv[2]` has to be a null pointer. — too honest for this site, Dec 17 '15 at 14:58
@Olaf The [link](http://www.iso-9899.info/n1570.html#5.1.2.2.1) in my previous comment: `argv[argc] shall be a null pointer.`. — Holt, Dec 17 '15 at 15:00
@Holt "argv[argc] shall be a null pointer. — If the value of argc is greater than zero, ..." So with `argc == 0`, `argv[0]` is the null pointer. IOWs C does not require "argv[0] is always the name of the program". Further "**If the value of argc is greater than zero**, the string pointed to by argv[0] represents the program name;" — chux - Reinstate Monica, Dec 17 '15 at 15:01
@chux You are right, I realized my mistake a bit after posting my comment... — Holt, Dec 17 '15 at 15:04
@Holt: I stand corrected. Overread the line in the standard. (btw: I asked for a pointer to the standard, i.e. the authoritative reference). FYI: http://port70.net/~nsz/c/c11/n1570.html#5.1.2.2.1p2 2nd item. — too honest for this site, Dec 17 '15 at 15:06
By the way, why are you using `strchr` like that instead of e.g. [`strtok`](http://en.cppreference.com/w/c/string/byte/strtok)? — Some programmer dude, Dec 17 '15 at 15:28
As commented below your answer, I'm modifying a well established tool, i.e, the tool was already using `strchr`. — drZaius, Dec 17 '15 at 15:49

Some programmer dude · Accepted Answer · 2015-12-17T15:26:47.790

You make argv[i] point to a string literal, which is an array of read-only characters. Then you attempt to modify this read-only array leading to undefined behavior.

There's a reason you should be using const char * for string literals.

There are a couple of ways to solve your problem. The first and simplest, especially if you are going to use the same string for all arguments (not very likely except in a contrived examples such as the one you show) is to use an array, like char argument[] = "bar=foo";. Relying on the natural decay of arrays to pointers to their first arguments, you could use that in the assignment to argv[i] instead, and modify the array to your hearts content.

But like I said that won't really be very useful except for simple examples. That leaves us with another option, using e.g. strdup to dynamically allocate and copy the strings (alternatively if strdup is not available, it's not a standard function, you could manually use malloc and strcpy). Since you then have dynamically allocated the memory it can also be modified to your hearts content. The problem here is that you then need to keep track of the original pointers and use free to free them again.

So these are basically the two solutions you could use.One is unrealistic, and the other have dynamic memory allocations and all the problem that entails. There are really no good and especially no simple solutions to your problem.

I see your point, but I'm not sure how to use `const char*` then to modify `argv[i]`? — drZaius, Dec 17 '15 at 15:11
@drZaius You can't, it's was mostly that when dealing with string literals you really need to think about their read-only aspects. I did however edit the answer to include a couple of possible solutions. — Some programmer dude, Dec 17 '15 at 15:27
True, I was actually modifying a tool that's available to every Unix machine (but shall remain nameless). Right before you further explained, I tried using an array and it actually did the job! So no seg fault anymore and my modification is up and running. — drZaius, Dec 17 '15 at 15:46

score 3 · Answer 2 · answered Dec 17 '15 at 14:52

You are writing to a string literal here

val = strchr(arg, '=');
*val = '\0';

String literals live in the read only segment of the program, and trying to alter them invokes undefined behavior.

After this line

argv[i] = "bar=foo";

argv[i] points to a string literal "bar=foo" you then, create a new pointer arg that points to argv[i] and finally val a poitner to the = in arg which you then go and try to overwrite with a nul.

C segmentation fault after altering argv

2 Answers2