My xstrcmp (which is copy of strcmp library function) gives incorrect results

Question

The compiler gives no error but I am not getting desired output of self made strcmp function.

#include <stdio.h>
#include<string.h>
    
    int main() {
    char str1[20],str2[20];
    gets(str1);
    gets(str2);
    printf("\n%d",xstrcmp(str1,str2));
        return 0;
    }
    int xstrcmp(char *p1,char *p2){
         int k;
        while(*p1!='\0'||*p2!='\0'){
            if((*p1-*p2)==0){
                p1++,p2++;
                continue;
            }
            else{
               
               k= ((*p1)-(*p2));
            }
        }
        return k;
    }

The compiler *should* warn about implicitly declared `xstrcmp`. Try turning up compiler warnings. Then either add a forward declaration, or simply move the function to be defined before you call it. — hyde, Jun 26 '22 at 17:36
do not use `gets`, but if you do, realize that everybody will tell you "don't use gets", and at least make the buffers much larger, like 1000 characters. — hyde, Jun 26 '22 at 17:37
@hyde I agree that this is a good thing. However, in this case, the C compiler will assume `xstrcmp` is a function return `int` (which it actually is) and "deduce" the two arguments correctly as `char*`. — Adrian Mole, Jun 26 '22 at 17:38
You also need to give `k` an initial value (which will be returned if the loop completes without finding a mismatch). Presumably, that value will be zero. — Adrian Mole, Jun 26 '22 at 17:41

Adrian Mole · Accepted Answer · 2022-06-26T18:02:04.653

First and foremost, please read this: Why is the gets function so dangerous that it should not be used?. So, instead of your gets(str1) line, use fgets(str1, 20, stdin) (and likewise for reading str2). Second, you should really provide a prototype (pre-declaration) of your xstrcmp before using it; a line like this before main will do: int xstrcmp(char* p1, char* p2); (or you could move the whole body of that function to before main). In your case, as the compiler will assume that it is a function returning an int, there is no major issue; but it's bad practice to use such undeclared functions and (IIRC), the option is planned for removal in a future C Standard.

Now to the logic problems in your function: There are two.

You don't give k any value that will be used if the while loop finished without finding a mismatch (i.e., if the passed strings are the same); this is a simple fix: initialize k to zero, like int k = 0;.
When you do find a mismatch, you neither increment the p1 and p2 pointers, and nor do you break out of the while loop – so that loop will continue forever, once such a mismatch is found. To fix this, simply add a break; statement in your else block.

Here is a working version of your code that addresses these issues:

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

int xstrcmp(char* p1, char* p2); // Pre-declaration of the function

int main() {
    char str1[20], str2[20];
    fgets(str1, 20, stdin);
    fgets(str2, 20, stdin);
    printf("\n%d", xstrcmp(str1, str2));
    return 0;
}

int xstrcmp(char* p1, char* p2) {
    int k = 0; // We need to give k a value to use if we complete the loop without a mismatch
    while (*p1 != '\0' || *p2 != '\0') {
        if ((*p1 - *p2) == 0) {
            p1++, p2++;
            continue;
        }
        else {
            k = ((*p1) - (*p2));
            break; // Once we find a mismatch, we must terminate the loop after assiging k
        }
    }
    return k;
}

Note: it is more conventional to use a loop condition like while (*p1 != '\0' && *p2 != '\0') (using && instead of ||). However, in your case, the internal loop logic (once corrected) will allow either to work: any mismatch will stop (break) the loop and, if the strings are the same, then both p1 and p2 will point to nul characters at the same time, so the test using || will still fail when it is expected to.

My xstrcmp (which is copy of strcmp library function) gives incorrect results

1 Answers1