Invoking the function renders an error; what explains this?

Question

A function I created, which I refer to as string_deletion, deletes occurrences of a specific substring and shifts the string leftwards by the length of the substring. The function accepts two parameters: the pointer to the location in the array wherein the first letter of the substring is encountered, and the length of the word.

Here is the function:

void string_deletion(char *s, int m)
{
    char *index=s+m;
    while(*index!=0)
    {
        *(index-m)=*(index++);
    }
    *(index-m)=0;
}

The function shifts all the characters after the substring leftwards by an amount dependent on the length of the substring, which has been denoted by m. I have set the index pointer to point to the character that occurs immediately after the occurrence of the substring, and this is the mark from where the shifting commences. The loop is executed until a NUL character is encountered, after which it exits the loop. At the culmination, the NUL is appended to the end of the string.

Whereas the other parts of the main code work seamlessly, invoking this specific function, as and when necessary, makes the program cease working and it yields an error. What explains this?

Here's the complete code:

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

int string_len(char *s)
{
    int i=0;
    char *m=s;
    while(*m!=0)
    {
        i++;
        m++;
    }
    return i;
}
void word_enter(char *word_search)
{
    char r;
    char *m=word_search;
    while((r=getchar())!=EOF)
    {
        if(r=='\n')
        {
            *m=0;
            break;
        }
        else
        {
            *(m++)=r;
        }
    }
}
 
void string_disp(char *s)
{
    char *d=s;
    while(*d!=0)
    {
        putchar(*(d++));
    }
}

int string_comp(char *s, char *m)
{
    int stringlength_one=string_len(s);
    int stringlength_two=string_len(m);
    char *s_one=s;
    char *s_two=m;
    if(stringlength_one!=stringlength_two)
    {
        return 1;
    }
    else
    {
        while(*s_one!=0)
        {
            if(*s_one!=*s_two)
            {
                return 1;
            }
            s_one++;
            s_two++;
        }
    }
    return 0;
}

void string_deletion(char *s, int m)
{
    char *index=s+m;
    while(*index!=0)
    {
        *(index-m)=*(index++);
    }
    *(index-m)=0;
}

void string_search(char *s,char *d)
{
    char *m=s;
    char word_buffer[20];
    char *buffer_index=word_buffer;
    while(m!=&s[string_len(s)-string_len(d)+1])
    {
        buffer_index=word_buffer;
        if(*m==*d)
        {   
            int i=0;
            char *r=m;
            while(i<=string_len(d) && *r!=0)
            {
                *(buffer_index++)=*(r++);
                i++;
            }
            *buffer_index=0;
            if(string_comp(word_buffer,d)==0)
            {
                printf("\nInvoking deletion sequence\n");
                string_deletion(m,string_len(d));
            }
        }
    m++;
    }
}   
    
int main(void)
{
    int pos;
    char main_string[100],word[20];
    printf("Enter the main string: ");
    word_enter(main_string);
    printf("\nEnter the string you wish to delete: ");
    word_enter(word);
    string_search(main_string,word);
    string_disp(main_string);
    exit(EXIT_SUCCESS);
}

Please show a [mcve]. The problem might be in the calling code. — Jabberwocky, Sep 06 '21 at 13:34
You probbaly stumbled on this: https://stackoverflow.com/questions/8718740/why-cant-i-write-to-a-string-literal-while-i-can-write-to-a-string-object. But without further information nobody can be sure. — Jabberwocky, Sep 06 '21 at 13:45
Are you, perchance, passing a string literal to your function? That is, do you have something like: `char* test = "I am a string literal";` and then use `test` ... which means you're trying to modify an immutable (constant) string. — Adrian Mole, Sep 06 '21 at 13:45
@AdrianMole, no, I didn't. I created a character array which accepts characters into it, and this is precisely what is passed onto the string_deletion function. — LordObnoxious, Sep 06 '21 at 13:52
@Jabberwocky, I have attached the minimum reproducible code. Please vet it. — LordObnoxious, Sep 06 '21 at 13:54
Please note that a [mre] should be *minimal*. Dumping your entire codebase here is not appropriate. — n. 1.8e9-where's-my-share m., Sep 06 '21 at 14:18
`while((r=getchar())!=EOF)` This is wrong (but not related to your problem). `getchar` returns an `int`, not a `char`. A `char` might not be able to hold the value `EOF` at all. — Gerhardh, Sep 06 '21 at 15:01
@Gerhardh, do you suggest using wchar_t datatype instead of char? — LordObnoxious, Sep 06 '21 at 17:28
`getchar` returns an `int`. Not a `wchar_t`. You should use `int`, not `wchar_t`. — n. 1.8e9-where's-my-share m., Sep 06 '21 at 20:50
I mentioned that `int` is the return type. What makes you beliefe `wchar_t` would be a good type to deal with `int` values? — Gerhardh, Sep 07 '21 at 06:05
@Gerhardh, I intend to store the returned value into a character array, owing to which I am typecasting the characters into the char type. — LordObnoxious, Sep 08 '21 at 03:04
There is nothing wrong with storing it in a `char[]`. But for checking if you got `EOF` you must use `int` first. — Gerhardh, Sep 08 '21 at 06:36

score 6 · Accepted Answer · answered Sep 06 '21 at 14:07

6

*(index-m)=*(index++)

This is undefined behaviour. If you use a post- or pre-inc/decrement, don't use the same variable in the same expression again.

index[-m] = *index;
++index;

answered Sep 06 '21 at 14:07

n. 1.8e9-where's-my-share m.

102,958
14
123
225

Good spot! MSVC missed that but clang-cl saw it. – Adrian Mole Sep 06 '21 at 14:13
In this instance, *(index-m) is equivalent to index[-m], is it? – LordObnoxious Sep 06 '21 at 15:15

Adrian Mole · Answer 2 · 2021-09-06T14:15:17.377

3

The problem (or, at least, one problem) is in your string_search function. The substring you extract from the given s argument is one character too long; thus, you won't get a match with the given d argument, unless you are very lucky.

To fix the problem, change the test condition in the while loop from i <= string_len(d) to i < string_len(d), like this:

void string_search(char* s, char* d)
{
    char* m = s;
    char word_buffer[20];
    char* buffer_index = word_buffer;
    while (m != &s[string_len(s) - string_len(d) + 1]) {
        buffer_index = word_buffer;
        if (*m == *d) {
            int i = 0;
            char* r = m;
            while (i < string_len(d) && *r != 0) { // Use i < string_len ... not i <=
                *(buffer_index++) = *(r++);
                i++;
            }
            *buffer_index = 0;
            if (string_comp(word_buffer, d) == 0) {
                printf("\nInvoking deletion sequence\n");
                string_deletion(m, string_len(d));
            }
        }
        m++;
    }
}

Also, be sure to address the issue of undefined behaviour highlighted in this answer (my MSVC compiler didn't spot that one, but clang-cl did)!

edited Sep 06 '21 at 14:15

answered Sep 06 '21 at 14:09

Adrian Mole

43,040
110
45
72

Um, how so? For instance, if the substring I intend to remove is 5 characters long, then I suppose the buffer will input 5 characters as well, won't it? – LordObnoxious Sep 06 '21 at 15:22
1

Yes. But you start counting at **zero** (as you should); so, when `i == 5` you have copied **six** characters. Try what I did: put a line at the top of your `string_comp` function that prints out the two strings it is given - you'll see that the first is one character longer than the second. – Adrian Mole Sep 06 '21 at 15:25
This fixes one bug. The answer from @n. 1.8e9-where's-my-share m., specifically addresses the bug in function `string_deletion`. – Jonathon S. Sep 06 '21 at 15:26
1

@JonathonS. Yes, indeed. That's why I added the footnote, linking to that answer. I *could* have included an explanation myself, but that would have been paramount to plagiarism, as they posted their answer before I even realized that bug was there. – Adrian Mole Sep 06 '21 at 15:27
@AdrianMole, that makes sense. How silly of me! – LordObnoxious Sep 06 '21 at 15:44

score 1 · Answer 3 · answered Sep 06 '21 at 14:44

1

I would not reinvent the wheel:

char *string_deletion(char *s, size_t m)
{
    memmove(s, s + m, strlen(s+m) + 1);
    return s;
}

answered Sep 06 '21 at 14:44

0___________

47,100
4
27
62

Invoking the function renders an error; what explains this?

3 Answers3