5

For unknown reason, result of running my C program is quite unexpected. I think that it has to be some kind of a beginner mistake, however I can't find out really, where is it.

#include <stdio.h>
#include <string.h>
int main()
{
char string1[50];
char string2[50];
int compare;

puts("Enter two strings: ");
fgets(string1, strlen(string1)+1, stdin);
fgets(string2, strlen(string2)+1, stdin);

compare=strcmp(string1, string2); /* usage of extra variable makes the code more readable but wastes more memory */

printf("%d: ",compare);

if (compare<0) puts("First string is lesser");
else if (compare>0) puts ("First string is bigger");
     else puts("Strings are equal");


return 0;
  }

And on testing:

Enter two strings: 
heheisntthisalongstring
heheisntthisalongstring
1: First string is bigger


------------------
(program exited with code: 0)
Press return to continue

Shouldn't those strings be equal?

hakre
  • 193,403
  • 52
  • 435
  • 836
hardpenguin
  • 159
  • 1
  • 2
  • 9
  • 3
    Your fgets() calls are borken, the length you pass isn't correct. An uninitialized char[] has a random length. Practice using a debugger to see these kind of problems. – Hans Passant Aug 24 '12 at 11:23
  • Thank you very much. I'll learn using debugger today. – hardpenguin Aug 24 '12 at 11:57
  • Just an extra question. After initialising arrays to 0 if I use strlen(), the program will end without giving me a chance to enter strings. Changing strlen() to sizeof solves the problem but why is above happening? – hardpenguin Aug 24 '12 at 12:03
  • 1
    @hardpenguin `fgets(s, num, stream)` reads characters until (num-1) characters have been read or either a newline or the End-of-File is reached, whichever comes first. When initializing your strings with all 0 `strlen` returns 0 and `fgets` has no characters to read. – halex Aug 24 '12 at 12:24

4 Answers4

13
fgets(string1, strlen(string1)+1, stdin);
fgets(string2, strlen(string2)+1, stdin);

These are wrong. string1 and string2 are not initialized, and strlen just counts the number of bytes, till hitting \0. In this case, strlen could return any (random non-negative) number.

Use sizeof, instead of strlen here.

Kiril Kirov
  • 37,467
  • 22
  • 115
  • 187
3

Here

 char string1[50]; 
 char string2[50]; 

you dont initialise them, so your initial calls to strlen are unreliable as they are looking for the first null char they find after the start of the array. This could be anywhere and the results of the call may or may not be a true reflection on the size - you just cant rely at all on the results.

mathematician1975
  • 21,161
  • 6
  • 59
  • 101
0

string1 is not memset to 0, So strlen(string1) value will not give the expected value (50). strlen will counts the character till it reaches \0. So it may leads to crash also(an undefined behaviour).

Better memset both string1 and string2 like below.

char string1[50] = {0}; 
char string2[50] = {0};

and also use sizeof operator to get the value 50.

fgets(string1, sizeof(string1), stdin); 
fgets(string2, sizeof(string2), stdin);

or else directly go for scanf

scanf("%s", string1);
scanf("%s", string2);
rashok
  • 12,790
  • 16
  • 88
  • 100
  • Memset zero on the array would not result in an "expected value of 50" when strlen is called on it – mathematician1975 Aug 24 '12 at 11:38
  • strlen(string1) will gives zero, if we pass memsetted string1. Thats what i suggested to use sizeof instead of strlen. Better all the buffer should be memset to 0 before using it. – rashok Aug 24 '12 at 11:40
  • memsetting all the buffer would affect the performance in a very bad way. – Abhineet Aug 24 '12 at 11:46
  • @raja ashok I agree it is just that your answer suggests that memsetting to zero will give a value of 50. – mathematician1975 Aug 24 '12 at 11:48
  • Thanks much, could you also answer this related: Just an extra question. After initialising arrays to 0 if I use strlen(), the program will end without giving me a chance to enter strings. Changing strlen() to sizeof solves the problem but why is above happening? – hardpenguin Aug 24 '12 at 12:07
  • 1
    @hardpenguin, sizeof() is compiler based. The compiler figures out the size of at that point and turns it into a constant. strlen() calculates the length of the string from where your pointer is till the next null value. Thats why strlen doesn't work in this case (because you memset everything to zero). – Youssef G. Aug 24 '12 at 12:19
0

Here have a look at this-strlen

Although the code you are using is not good but you can still get your expected answer by using strncmp giving the 3rd parameter the strlen of a common string variable. Just for fun. Always initialize your variables or they can lead your application to crash. You can see the examples here- strncmp

Abhineet
  • 5,320
  • 1
  • 25
  • 43
  • No initialization and usage of strncmp with same strlen variable can still crash your app. Just being clear, am not endorsing this code :-) – Abhineet Aug 24 '12 at 11:48