0

I am reading a URL (which is string) and searching for a pattern (consecutive occurrences of the / character). If I find a matching pattern, I want to replace it with a single / and copy rest of the characters as they are. For example: If the input string is http://www.yahoo.com/, I need to produce output http:/www.yahoo.com/ by removing the extra / since that character occurred twice, consecutively.

Here is the program:

int main() {
    int i, j;
    bool found = false;
    unsigned char *str = "http://www.yahoo.com/";
    int len = strlen(str);
    for (i = 0; i < len - 1; i++) {
        if ((str[i] == '/') && (str[i + 1] == '/')) {
            found = true;
            break;
        }
    }
    if (found) {
        for (j = i + 1; j <= (len - i - 2); j++) {
            str[j] = str[j + 1];
        }
    }
    return 0;
}

But this program is generating a segmentation fault. Where is the problem in this code? Any idea how to fix it? Any alternate simple implementation for this?

Walf
  • 8,535
  • 2
  • 44
  • 59
tinga
  • 53
  • 4
  • 2
    @tinga You may not change a string literal. Also there is standard function strstr that will help to find quickly the string "//" – Vlad from Moscow Dec 13 '16 at 01:21
  • 3
    It's a strange example, taking a valid URI and turning it into an invalid one. – paddy Dec 13 '16 at 01:28
  • Possible duplicate of [Why is this string reversal C code causing a segmentation fault?](http://stackoverflow.com/questions/1614723/why-is-this-string-reversal-c-code-causing-a-segmentation-fault) – John3136 Dec 13 '16 at 03:15

2 Answers2

2

You may not change string literals. They are unmodifiable in C and C++. According to the C Standard (6.4.5 String literals)

7 It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.

The task can be easy done using standard C function strstr. For example

char s[] = "http://www.yahoo.com/";

puts(s);

char *p = strstr(s, "//");

if (p) memmove(p, p + 1, strlen(s) - (p - s));

puts(s);

The output of the code snippet will look like

http://www.yahoo.com/
http:/www.yahoo.com/

As for your program then apart from an attempt to change a string literal this loop is wrong

    if (found) {
        for(j = i + 1; j <= (len - i - 2); j++) {
            str[j] = str[j + 1];
        }
    }

It should look at least like

    if (found) {
        for(j = i + 1; j < len; j++) {
            str[j] = str[j + 1];
        }
    }
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

You are operating on a string literal, which is read-only memory. When you try to modify the characters, you get an error.

Copy your string data into writable memory and then you can modify it.

The simplest change is to make this line:

unsigned char *str = "http://www.yahoo.com/";

Be this instead:

char str[] = "http://www.yahoo.com/";

However, for C++, you should use a std::string instead, and then you can use standard search algorithms, like this:

#include <string>

int main() {
    std::string str = "http://www.yahoo.com/";
    std::string::size_type i = 0;
    do {
        i = str.find("//", i);
        if (i == std::string::npos) break;
        str.erase(i, 1);      
    }
    while (!str.empty());
    return 0;
}

Alternatively:

#include <string>
#include <algorithm>

bool isBackslashPair(const char c1, const char c2) {
    return ((c1 == '/') && (c2 == '/'));
}

int main() {
    std::string str = "http://www.yahoo.com/";
    std::string::iterator iter = str.begin();
    do {
        iter = std::adjacent_find(iter, str.end(), isBackslashPair);
        if (iter == std::string::end()) break;
        iter = str.erase(iter);      
    }
    while (!str.empty());
    return 0;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770