C++ program crashes

Question

I've this assignment to implement strcmp function. Sometimes it runs okay but other times it crashes. Please help me.

#include <iostream>

using namespace std;     

int mystrcmp(const char *s1, const char *s2);

int main()
{
cout<<mystrcmp("A","A")<<endl;     
cout<<mystrcmp("B","A")<<endl;     
cout<<mystrcmp("A","B")<<endl;     
cout<<mystrcmp("AB","A")<<endl;

return 0;     
}

int mystrcmp(const char *s1, const char *s2)
{
 while (*s1==*s2)
 {
  s1++;
  s2++;
 }

 if(*s1=='\0')
  return(0);

 return(*s1-*s2);
}

Answer 1

Your mystrcmp will happily run off the end of the string, because your test for the NUL terminator is outside the loop. If the strings are the same, then both *s1 and *s2 are 0 and the loop keeps going.

Answer 2

while (*s1==*s2)
{
 s1++;
 s2++;
}

'\0' == '\0'

Answer 3

It will crash if both the input are identical, because your loop continues beyond the terminating nul character.

To fix this you must have a check for nul character inside the loop as:

while (*s1==*s2) {

  // if s1 points to nul character, then s2 should also, because of the ==
  // which means we've reached the end of the strings and they are equal
  // so return 0.
  if(*s1=='\0')
    return 0;

  s1++;
  s2++;
 }

 return *s1-*s2;

Answer 4

You need to think what will happen if your two strings are as follows:

s1:this is a string\0|abcdef
s2:this is a string\0|abcdef
       good memory <-|-> bad memory

Because you're simply advancing the pointers while the contents are equal, you may end up reading memory in a way that's undefined.

A better way is to base your code of the following pseudo-code:

def strcmp(s1,s2):
    while character at s1 is not '\0':
        if character at s1 is not the same as character at s2:
            exit while loop
        increment s1 and s2
    return difference between *s1 and *s2

This will stop when you either reach the end of the first string or you find a difference (including if you've reached the end of the second string before the first).