CODEWITHSUNDEEP
pointersstrcmp
user1215630
user1215630

Reputation:

slow execution of string comparision

my problem why my program takes much large time to execute, this program is supposed to check the user password, the approach used is take password form console in to array and compare it with previously saved password comparision is done by function str_cmp()-returns zero if strings are equal,non zero if not equal

  #include<stdio.h>
  char str_cmp(char *,char *);
  int main(void)
  {
      int i=0;
      char c,cmp[10],org[10]="0123456789"; 
      printf("\nEnter your account password\ntype 0123456789\n");
      for(i=0;(c=getchar())!=EOF;i++)
      cmp[i]=c;
      if(!str_cmp(org,cmp))
      {
          printf("\nLogin Sucessful");  
      }
      else
          printf("\nIncorrect Password");
  return 0;
  }
  char str_cmp(char *porg,char *pcmp)
  {
    int i=0,l=0;
    for(i=0;*porg+i;i++) 
    {
        if(!(*porg+i==*pcmp+i))
        {
          l++;
        }        
    }
  return l;
  }

Upvotes: 0

Views: 77

Answers (2)

drewag
drewag

Reputation: 94773

There are libraries available to do this much more simply but I will assume that this is an assignment and either way it is a good learning experience. I think the problem is in your for loop in the str_cmp function. The condition you are using is "*porg+i". This is not really doing a comparison. What the compiler is going to do is go until the expression is equal to 0. That will happen once i is so large that *porg+i is larger than what an "int" can store and it gets reset to 0 (this is called overflowing the variable).

Instead, you should pass a size into the str_cmp function corresponding to the length of the strings. In the for loop condition you should make sure that i < str_size.

However, there is a build in strncmp function (http://www.elook.org/programming/c/strncmp.html) that does this exact thing.

You also have a different problem. You are doing pointer addition like so:

*porg+i

This is going to take the value of the first element of the array and add i to it. Instead you want to do:

*(porg+i)

That will add to the pointer and then dereference it to get the value.

To clarify more fully with the comparison because this is a very important concept for pointers. porg is defined as a char*. This means that you have a variable that has the memory address of a 'char'. When you use the dereference operator (*, for example *porg) on the variable, it returns the value at stored in that piece of memory. However, you can add a number to the memory location to move to a different memory location. porg + 1 is going to return the memory location after porg. Therefore, when you do *porg + 1 you are getting the value at the memory address and adding 1 to it. On the other hand, when you do *(porg + 1) you are getting the value at the memory address one after where porg is pointing to. This is useful for arrays because arrays are store their values one after another. However, a more understandable notation for doing this is: porg[1]. This says "get the value 1 after the beginning of the array" or in other words "get the second element of the array".

All conditions in C are checking if the value is zero or non-zero. Zero means false, and every other value means true. When you use this expression (*porg + 1) for a condition it is going to do the calculation (value at porg + 1) and check if it is zero or not.

This leads me to the other very important concept for programming in C. An int can only hold values up to a certain size. If the variable is added to enough where it is larger than that maximum value, it will cycle around to 0. So lets say the maximum value of an int is 256 (it is in fact much larger). If you have an int that has the value of 256 and add 1 to it, it will become zero instead of 257. In reality the maximum number is 65,536 for most compilers so this is why it is taking so long. It is waiting until *porg + i is greater than 65,536 so that it becomes zero again.

Upvotes: 3

Jeff Lamb
Jeff Lamb

Reputation: 5865

Try including string.h:

#include <string.h>

Then use the built-in strcmp() function. The existing string functions have already been written to be as fast as possible in most situations.

Also, I think your for statement is messed up:

for(i=0;*porg+i;i++)

That's going to dereference the pointer, then add i to it. I'm surprised the for loop ever exits.

If you change it to this, it should work:

for(i=0;porg[i];i++)

Your original string is also one longer than you think it is. You allocate 10 bytes, but it's actually 11 bytes long. A string (in quotes) is always ended with a null character. You need to declare 11 bytes for your char array.

Another issue:

if(!(*porg+i==*pcmp+i))

should be changed to 

if(!(porg[i]==pcmp[i]))

For the same reasons listed above.

Upvotes: 1

Related Questions