2

for school, I have to recreate the strstr function. I did, and I tested it as much as I could, and everything seems to work perfectly.

I am still looking into it but after several hours of troubleshooting, I don't get it. Can someone tell me what's wrong with it compared to strstr ?

char    *ft_strstr(char *str, char *to_find)
{
    int i;
    int k;

    i = 0;
    k = 0;
    if (*to_find == 0)
        return (str);
    while (str[i] != 0)
    {
        while (str[i] == to_find[k])
        {
            i++;
            k++;
            if (to_find[k] == 0)
                return (&str[i - k]);
        }
        i++;
        k = 0;
    }
    return (0);
}

Should I include a main for testing ?

Thank you.

EDIT : Here is the error code of the machine :

> $> ./0z09k44vyodiwxihua4w30m9 $> diff -U 3 user_output_test1
> test1.output | cat -e
> --- user_output_test1   2021-08-11 17:55:47.000000000 +0000$
> +++ test1.output        2021-08-11 17:55:47.000000000 +0000$ @@ -2,7 +2,7 @@$  0$  -1$  0$
> -7$
> +-1$  0$  2$  2$ @@ -12,7 +12,7 @@$  -1$  -1$  -1$
> -87$
> +-1$  -1$  -1$  -1$
> 
> Diff KO :( Grade: 0
mediocrevegetable1
  • 4,086
  • 1
  • 11
  • 33
Toyama70
  • 23
  • 3
  • 2
    The problem is that if the inner while-loop matches the first few characters of `to_find`, but not the later ones, you will have incremented `i` too much for the next iteration of the outer while-loop. – G. Sliepen Aug 11 '21 at 18:21
  • 1
    Off-topic but you should have both your parameters as `const char *` instead since they should not be modified. – mediocrevegetable1 Aug 11 '21 at 18:22
  • and then don't increment `i` but test `str[i + k]` and then the match will be at `str + i`. – Weather Vane Aug 11 '21 at 18:23
  • 1
    Try `str = "ababac", to_find = "abac"`. Unrelated to your problem, `return` doesn't need parentheses, `int i; i = 0;` can be combined to `int i = 0;`, and it's type should be changed to `size_t`. The parameters should have type `const char *`. – HolyBlackCat Aug 11 '21 at 18:24
  • Aside: don't set `k = 0` twice where you don't need it, set it once just before where you *do* need it. That is just before the second `while`. – Weather Vane Aug 11 '21 at 18:25
  • Thank you guy, indeed there is a problem with the example "ababac", reading your answers and trying to fix it. – Toyama70 Aug 11 '21 at 18:30
  • 1
    "a program designed to test it" I hope you have privileges to run that program and that each test that's executed has a name and that when a test fails it says what it expects and what it got instead. In other words, if you can't run it or if it renders only a scalar pass-fail result, then you should consider at least talking to your instructor. There is *so* much to learn in the domain of testing. – Jeff Holt Aug 11 '21 at 18:30
  • 3
    Something to remember: when dealing with something even only halfway reputable that says your code is wrong and you think your code is correct: always start with your code being wrong, and not "My code is right. The test program/compiler/whatever is wrong." Guess what - chances are your code is wrong and you just don't see it. And the more users for whatever says your code is wrong, the more likely your code actually is wrong. If GCC says your code is wrong, 99.99999999% of the time, your code will be wrong. A test run by your professor? Probably 99.99% of the time your code will be wrong. – Andrew Henle Aug 11 '21 at 18:37
  • do carry on, but if you absolutely get jammed up it's no secret the source code for `strstr` implementations is readily available... – yano Aug 11 '21 at 18:38
  • It's done, sorry. I am trying to understand how to fix it now that indeed it appears that it doesn't work. So what exactly should I do to make it work ? Sorry for asking in such a straight way. – Toyama70 Aug 11 '21 at 18:38
  • @Toyama70 Why are not you using standard string functions as for example strlen? – Vlad from Moscow Aug 11 '21 at 18:39
  • @VladfromMoscow He's been given an assignment by his instructor, who apparently doesn't like to write straightforward, easy-to-understand tests. – Jeff Holt Aug 11 '21 at 18:39
  • It's [here](https://stackoverflow.com/questions/68747133/my-strstr-function-works-but-a-program-designed-to-test-it-says-it-doesnt#comment121495751_68747133) - don't increment `i` inside the inner loop, but work with `i + k`. – Weather Vane Aug 11 '21 at 18:41
  • We are restricted and cannot use any function we didn't code ourselves. It's School 42 so no instructor to ask :/ – Toyama70 Aug 11 '21 at 18:41
  • Thanks, gonna try to work with i+k – Toyama70 Aug 11 '21 at 18:42
  • Did you try to figure out what the input is for the case where the test fails your program? Do you know what the output should be in that case? Does your program give that output? When you did your own tests, what test cases did you use? – Karl Knechtel Aug 11 '21 at 19:12
  • [test](https://godbolt.org/z/aPGP8bcG8) – n. m. could be an AI Aug 11 '21 at 19:18

4 Answers4

2

These while loops are incorrect

while (str[i] != 0)
{
    while (str[i] == to_find[k])
    {
        i++;
        k++;
        if (to_find[k] == 0)
            return (&str[i - k]);
    }
    i++;
    k = 0;
}

Let's assume that str is "aab" and to_find is "ab". In this case the inner while loop gets the control because str[0] is equal tp to_find[0]. Within the loop the both indices are incremented and str[1] is not equal to to_find[1]. So the loop interrupts its iteration and after the loop the index i is incremented again and becomes equal to 2. So the substring will not be found.

You need to introduce another index for the inner while loop. For example

while (str[i] != 0)
{
    int j = i;
    while (str[j] == to_find[k])
    {
        j++;
        k++;
        if (to_find[k] == 0)
            return (&str[j - k]);
    }
    i++;
    k = 0;
}

In fact you need to use only one variable as an index. The loops can be rewritten like

for ( ; *str; ++str )
{
    size_t i = 0;

    while ( str[i] == to_find[i] )
    {
        if ( to_find[++i] == '\0' ) return str;
    }
} 

Pay attention to that the function has the following declaration

char *strstr(const char *s1, const char *s2);

That is its parameters have the qualifier const.

If you will declare the function this way then the return statement will look like

return ( char * )(&str[j - k]);

or

return ( char * )str;

if to use the modified loops shown above.

Here is a demonstrative program.

#include <stdio.h>

char * ft_strstr( const char *str, const char *to_find )
{
    if ( *to_find == '\0' ) return ( char * )str;
    
    for ( ; *str; ++str )
    {
        size_t i = 0;

        while ( str[i] == to_find[i] )
        {
            if ( to_find[++i] == '\0' ) return ( char * )str;
        }
    }
    
    return NULL;
}

int main(void) 
{
    char *p = ft_strstr( "aab", "ab" );
    
    if ( p ) puts( p );
    
    return 0;
}

The program output is

ab
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Thanks for taking the time to answer my question, I am gonna read that and try to fix it, although it's late here in Belgium so I am gonna handle that tomorrow. I will update the thread as soon as I managed to fix it ! But thank you again ! – Toyama70 Aug 11 '21 at 20:05
  • Hey ! Thank you for the super detailed and easy to understand answer, I perfectly got it ! Only small question, in the end, for the return statement, you mean that I can write it like this : return (&str[j - k]); ? – Toyama70 Aug 11 '21 at 20:26
  • @Toyama70 Yes, you can write either return &str[j - k]; or just return &str[i]; or return str + i;. If you will declare the function as it is declared in teh C Standard with the qualifier const for its parameters then you need to cast the return expression likefor example return ( char * )( str + i ); – Vlad from Moscow Aug 11 '21 at 20:33
1

The main problem is that your code needs to compare str to to_find for every starting index in str, until it finds a match. Currently, if your code find a partial match, it will "skip" checking certain starting indices. For example, try the string aab and the substring ab. It will actually result in a segfault.

Some additional advice:

  • Look up the exact function definition and behavior on a website like cppreference.
  • For i and k; It is better to define and initialize a variable on the same line.
  • Be explicit: return NULL (a null pointer) and compare with \0 for the null-terminator.
  • When thinking of test cases, do not think of a string and a substring, but of any valid input and how your program will handle it.
Yun
  • 3,056
  • 6
  • 9
  • 28
0

Your strstr() doesn't work. I'm having difficulty constructing the counterexample, but if you advance several places through to_find on a false match you don't rewind and start in the next place and try the match again.

As Weather Vane wrote in comments, don't increment i in the inner loop. That's what causes the problem.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Joshua
  • 40,822
  • 8
  • 72
  • 132
0

The main problem is you do not backtrack to the beginning of the match when you have a partial match: your function will not match aab in the string aaab. You can fix this by setting i = i - k + 1; instead of i++; after the inner loop.

Also note that 0 in your code means very different things: the null terminator byte in a string, the number zero as an index, and the null pointer. It would be more readable to use '\0' for the null terminator and NULL for the null pointer.

Furthermore, index variables should have type size_t in case string lengths exceed the range of type int.

Here is a modified version:

char    *ft_strstr(char *str, char *to_find)
{
    size_t i;
    size_t k;

    i = 0;
    k = 0;
    if (*to_find == '\0')
        return (str);
    while (str[i] != '\0')
    {
        while (str[i] == to_find[k])
        {
            i++;
            k++;
            if (to_find[k] == '\0')
                return (&str[i - k]);
        }
        i = i - k + 1;
        k = 0;
    }
    return (NULL);
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189