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');

Answer 1

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).

1. You are passing a non-string, something that's not terminated with a \0 character.

2. 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