error implementing strstr() function

Question

I have implemented strstr() but the code has is not working please help:

#include<stdio.h>
#include<conio.h>
#include<stdlib.h>
#include<string.h>
char* mystrstr(char*,char*);
int main()
{
    char *s1,*s2,*flag;
    printf("Enter a string:\n ");
    gets(s1);
    printf("Enter a word to find in it:\n");
    gets(s2);
    flag=mystrstr(s1,s2);
    if(flag)
        printf("Word found\n");
    else
        printf("Word not found");
    getch();
    return 0;
}
char* mystrstr(char* s1,char* s2)
{       int flag=0,j,i;
    char* temp;
    for(i=0;i<strlen(s1);i++)
     {  if(*(s1+1)==s2[0])
        {
            for(j=1;*s2;j++)
            {
            if(*(s1+i)==*(s2+j))
            flag=1;
            else
            flag=0;
            }
        }
     }
     if(flag)
     {
     temp=(char*)malloc(sizeof(char*));
     itoa(j,temp,10);
     return *temp;
     }
     return 0;


}

Answer 1

Some quick tips:

1. Allocate memory to pointers or use arrays
2. Read more on pointer arithmetic here
3. Read about strstr and think of an algorithm to do so or google
4. More on itoa, specifically read the return value.

Answer 2

for(i=0;i<strlen(s1);i++)
 {  

        for(j=0;strlen(s2);j++)//and here
        {
        if(*(s1+i+j)==*(s2+j)){//here
        flag=1;}
        else{
        flag=0;break}
        }
 if(flag){break;}

 }

Answer 3

Hi,

For starters, you might want to read on how 'gets' works: http://en.wikipedia.org/wiki/Gets

You're just declaring pointers on the stack in your main-method. These pointers will probably point to some random memory. As mentioned in the comments above you will have to allocate memory either on the heap using 'malloc' (s1 = malloc(256);) or on the stack with f.ex. 'char s1[256];' etc. and then passing the address to it to gets with 'gets(&s1);' instead

Answer 4

conio.h is not ANSI C

getch is not ANSI C

itoa is not ANSI C

Your function returns a string contains a positionnumber in s2. Is this right for you?

The return must be freed.

Do you know an array-access? Better than *(s2+j) is s2[j].

if(*(s1+1)==s2[0]) should be if(*(s1+i)==s2[0]) or not?

for(j=1;*s2;j++) should be for(j=1;s2[j];j++) or not?

In the case flag=0 a break is missing, see other answers.

Answer 5

Did you check for the first characters?

if(*(s1+1)==s2[0]) // you are checking s1[1] against s2[0]

This is infinite loop unless s2[0] is a null character in the first place or you break from the loop using break;

for(j=1;*s2;j++)

A recrusive approach for your reference:

char* my_strstr(char* s1, char* s2) {
  // return null pointer for empty strings
  if (strlen(s1) == 0 || strlen(s2) == 0)
    return 0;

  // recursive part
  if (s1[0] == s2[0]) {
    if (strlen(s2+1) == 0 || (strlen(s2+1) != 0 && my_strstr(s1+1, s2+1) == s1+1))
      return s1;
  }
  else return 0;
}