0

Here is my try to remove duplicates of a string, and I have two questions:

void removeDuplicates(char *original_string)
{
    if(original_string == NULL) {
        return;
    }
    int len = strlen(original_string);
    if (len < 2) {
        return;
    }

    int tail = 1;
    int i;
    for (i = 1; i < len; i++) {
        int j;
        for (j=0; j < tail; j++) {
            if (original_string[i] == original_string[j]) {
                break;
            }
        }
        if (j == tail) {
            original_string[tail] = original_string[i];
            ++tail;
        }
    }
}

First: What am I doing wrong that I don't see? I have found this example in a book and I believe it makes sense. Why are the duplicated characters not being deleted?

Second: When calling the function, if I do it with:

char duplicated[] = "aba";
removeDuplicates(duplicated);

I don't get an error. But if I do it with:

char *duplicated = "aba";
removeDuplicates(duplicated);

I get an Bus error: 10 in run time.

Hommer Smith
  • 26,772
  • 56
  • 167
  • 296
  • 1
    possible duplicate of [Why do I get a segmentation fault when writing to a string?](http://stackoverflow.com/questions/164194/why-do-i-get-a-segmentation-fault-when-writing-to-a-string) – P.P Jan 23 '13 at 19:16
  • My question is not just about the error, but why the function is not working. – Hommer Smith Jan 23 '13 at 19:19
  • I don't see any such mention in your question. You said `when I do the first one I don't get error` and error for the second one. You haven't said anything about the whether it works or not. Do you expect us to compile and verify the functionality? – P.P Jan 23 '13 at 19:23
  • You don't see this? First: What am I doing wrong that I don't see? I have found this example in a book and I believe it makes sense. Why are the duplicated characters not being deleted? I don't expect you to do anything if you don't want to. I can't see why my program is not working and I am asking for help. – Hommer Smith Jan 23 '13 at 19:26
  • Nobody answered for what say now because your question never seemed to suggest with that function itself. If so, you have to edit your question with that. – P.P Jan 23 '13 at 19:36

4 Answers4

4
char duplicated[] = "aba";

creates an array of chars, which is writable.

char *duplicated = "aba";

creates a string literal (which is unmodifiable) then the variable duplicated is assigned to the pointer to that string literal. Since your function tries to modify the string in-place, it invokes undefined behavior when attempting to write to a string literal, hence the crash.

  • But even declaring it as char duplicated[] = "aba"; I don't see the duplicated characters being deleted. Where is the mistake inside the function? – Hommer Smith Jan 23 '13 at 19:17
  • @HommerSmith Now that's another question. Have you tried debugging it? Also, what do you expect `original_string[i];` to do? It does nothing in itself. It's an expression statement without side effects whose value is immediately discarded - essentially a no-op. –  Jan 23 '13 at 19:18
  • It's in my original question... I ask two questions, the first one why is not working and the second one, why I get that error when declaring the string itself... – Hommer Smith Jan 23 '13 at 19:19
1

"..." creates a constant chunk of memory holding your string.
You cannot modify it.

Therefore, modifying original_string[tail] is undefined behavior when called on a constant string.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • `original_string[tail]` is not by itself undefined behavior. Otherwise, you couldn't use string literals in any (useful) way :) –  Jan 23 '13 at 19:16
  • Thanks for your question. Actually my question also included why is my method not working in removing duplicated characters. I fail to see what's happening. – Hommer Smith Jan 23 '13 at 22:08
1

string literals are non-modifiable in C. it is undefined behaviour

So, duplicated has to be a local array:

or it has to be

char duplicated[] = "aba";

and not 

char *duplicated = "aba";
sr01853
  • 6,043
  • 1
  • 19
  • 39
0

nothing is being removed

original_string[tail] = original_string[i]

that isn't removing anything, it's replacing

user2011260
  • 1