Remove a substring from a string with memmove in C

Question

I'm trying to implement a function for removing a substring from a string with memmove. When printing out the results, it seems like I have not moved correctly the substrings, even though it seems like I used the correct position in the source string. My function is:

char * removeSubStr(char * str, const char * substr){
    char *scan_p, *temp_p;
    int subStrSize = strlen(substr);
    if (str == NULL){
        return 0;
    }
    else if (substr == NULL){
        return str;
    }
    else if (strlen(substr)> strlen(str)){
        return str;
    }
    temp_p = str;
    while(scan_p = strstr(temp_p,substr)){
        temp_p = scan_p + subStrSize;
        memmove(scan_p, temp_p, sizeof(temp_p)+1);

    }
    return str;
}

My output, for example is: if sending the string "please remove rem remove rem999", I'm getting back: "please ove rm ovmove re 999"

Thanks!

(Unrelated to your problem; but you should test `substr == NULL` before you pass `substr` to `strlen`. Otherwise you might end up calling `strlen` on a null pointer.) — M Oehm, Aug 08 '17 at 12:34

Mike Nakis · Answer 1 · 2017-08-08T13:05:30.580

The following:

while(scan_p = strstr(temp_p,substr)){
    temp_p = scan_p + subStrSize;
    memmove(scan_p, temp_p, sizeof(temp_p)+1);
}

makes very little sense.

You need something like this:

while( temp_p = strstr( temp_p, substr ) )
{
    length = strlen( temp_p );
    memmove( temp_p, temp_p + subStrSize, length - subStrSize + 1 );
}

Note: in the first version of my answer I was just using strlen(), but as commenters pointed out, that would be inadvisable because of the standard. (It would still most probably work, since we are copying down, not up, but it is best not to tempt your fate by violating the standard.) So, that's why we need memmove().

Note that with a couple of more lines of code this can be optimized so that you don't have to compute length = strlen( temp_p ); in each iteration of the loop. This optimization is left as an exercise to the student.

Also please note:

You better do int subStrSize = strlen(substr); after you check substr == NULL;
There is no such thing as styrlen()
sizeof(temp_p) does something completely different from what you think it does.

`strcpy( temp_p, temp_p + subStrSize );` is undefined behavior. Per **7.24.2.3 The `strcpy` function** of [the C standard](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf): "If copying takes place between objects that overlap, the behavior is undefined." — Andrew Henle, Aug 08 '17 at 12:45
@AndrewHenle oh, I did not know that. I will fix this. Thanks. — Mike Nakis, Aug 08 '17 at 12:46

score 1 · Answer 2 · answered Aug 08 '17 at 13:42

In this loop

temp_p = str;
while(scan_p = strstr(temp_p,substr)){
    temp_p = scan_p + subStrSize;
    memmove(scan_p, temp_p, `sizeof(temp_p)`+1);

}

there are two bugs.

The first one is that after each iteration of the loop the pointer temp_p should be equal to the value of the pointer scan_p because the tail of the string is moved in this position.

The second one is that this expression

sizeof(temp_p)

yields the size of an object of the type char * instead of providing the length of the string pointed to by the pointer temp_p.

Also your function too often calls the function strlen.

As for the function design in whole then the function should not check whether one of the parameters is equal to NULL. It is the task of the client of the function. Standard C string functions do not do such a check.

The function implementation can look the following way as it is shown in the demonstrative program.

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

char * removeSubStr( char *str, const char *substr )
{
    size_t m1 = strlen(str);
    size_t m2 = strlen(substr);

    if (!(m1 < m2))
    {
        for (char *p = str; (p = strstr(p, substr)) != NULL; )
        {
            size_t n = m1 - ( p + m2 - str );
            memmove(p, p + m2, n + 1);
        }
    }

    return str;
}

int main( void )
{
    char s[] = "12A12B12C12D";

    puts(s);

    puts(removeSubStr(s, "12"));

    return 0;
}

The program output is

12A12B12C12D
ABCD

score -1 · Answer 3 · answered Aug 08 '17 at 12:39

-1

sizeof(temp_p) is the size of the char *, usually 4

You probably wanted to write strlen(temp_p)

answered Aug 08 '17 at 12:39

Nimrod Morag

938
9
20

strlen returns the length without the '\n', and sizeof includes the '\n' inside. isn't it? – ILG Aug 08 '17 at 12:43

Remove a substring from a string with memmove in C

3 Answers3