Why does this code raise an Access violation while writing exceptions?

Question

Does anyone know why the following code raises an access violation writing exception?

void squeeze_c(char s[], int c)
{
    int i, j;

    for (i = j = 0; s[i] != '\0'; i++)
    {
        if (s[i] != c)
            s[j++] = s[i];
        s[j] = '\0';
    }
}

I called the function as following:

squeeze_c("Test", 's');

How are you calling the function? What do you pass for arguments? — Some programmer dude, Feb 22 '15 at 04:21
If you pass a string literal to this function, then it will probably crash. — Jonathan Leffler, Feb 22 '15 at 04:22
I'm going to bet that this is a duplicate of http://stackoverflow.com/questions/1614723/why-is-this-string-reversal-c-code-causing-a-segmentation-fault — Bill Lynch, Feb 22 '15 at 04:22
i passed squeeze_c("Test", 's'); It raised the exception once i and j are both 0 — Jeffrey Ye, Feb 22 '15 at 04:27
String literals are *constant*, you are not allowed to modify them. — Some programmer dude, Feb 22 '15 at 04:29
but if i changed from s[j++] = s[i]; to s[j] = s[i]; j++; but it still failed — Jeffrey Ye, Feb 22 '15 at 04:31

paxdiablo · Answer 1 · 2015-02-22T04:38:36.237

There's nothing inherently wrong with that code so it comes down to two possibilities (probably more, but these are the most common in my experience).

You are passing a non-string, something that's not terminated with a \0 character.
You are passing a string literal, something you are not permitted to modify.

Now that you've specifically stated you're calling it with squeeze_c("Test", 's');, point two above is the correct one.

The former is if you do something like:

char str[] = {'n','o',' ','n','u','l',' ','h','e','r','e'};

and you can fix that by simply making certain it has a \0 at the end.

The latter is if you do something like one of:

squeeze_c("this is a literal", 'l');

or:

char *xyzzy = "another literal";
squeeze_c(xyzzy, 'l');

You can fix that by ensuring you have a writable copy of the literal, something like:

char xyzzy[] = "another literal";
squeeze_c(xyzzy, 'l');

And you really don't want to add the \0 inside the inner loop since it's likely to damage your source string.

Consider if you use:

char xyzzy[] = "paxdiablo";
squeeze_c (xyzzy, 'd');

The first time through your loop, you'll overwrite the p with itself and then overwrite the a with \0. Then you'll exit the loop because the next character found at index i will be that \0 you just wrote to the string).

So it won't so much remove all the d characters as truncate your entire string to a length of one.

Instead, you'd be better off with:

void squeeze_c(char *s, int c) {
    int i, j;
    for (i = j = 0; s[i] != '\0'; i++)
        if (s[i] != c)
            s[j++] = s[i];
    s[j] = '\0';  // moved outside the loop
}

This will transform the string as follows (with | representing \0):

paxdiablo|     original string
paxdiablo|     copy p over p
paxdiablo|     copy a over a
paxdiablo|     copy x over x
paxdiablo|     no copy, j now one less than i
paxiiablo|     copy i over d
paxiaablo|     copy a over i
paxiabblo|     copy b over a
paxiabllo|     copy l over b
paxiabloo|     copy o over l
paxiablo||     final placement of \0

Thanks, that's my typo error. The s[j] should be outside the loop. But the exception was raised before that statement. — Jeffrey Ye, Feb 22 '15 at 04:53

Why does this code raise an Access violation while writing exceptions?

1 Answers1