0

I am new to C and having trouble with the program below. It asks people if they are well or not and displays a message accordingly. So far I have tried using !strcmp, strcmp and strncmp and none returns a positive value for if, all skip to the else statement. Can any one point to me where I am going wrong as the syntax seems fine to me.

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

int main()
{
   char well[3];

   printf("Hello, are you well today? (Yes/No) ");
   scanf_s ("%s",well);
       if (!strcmp(well,"Yes")){
           printf("Glad to hear it, so am I\n");
   }
   else{
       printf("Sorry to hear that, I hope your day gets better\n");
   }   
system("PAUSE");   
return 0;
}

Many thanks to all for all the answers unfortunately none of them seem to work. Assigning a 4 to take account of the null makes no difference. Invoking scanf rather then scanf_s results in an access violation (which is odd as the rest of the program uses plain scanf. Adding a '4' parameter to scanf_s also makes no difference. Really tearing my hair out here I'm happy to accommodate the null at the end of the line but the program won't seem to recognise it.

user2346526
  • 1
  • 1
  • 3

6 Answers6

5

The string "Yes" includes a null terminator and looks like {'Y', 'e', 's', '\0'} in memory. It therefore requires 4 chars so can't be safely read into a 3-element char array so you should give well at least 4 elements

char well[4];

or, as suggested by lundin

#define MAX_RESPONSE (sizeof("Yes"))
char well[MAX_RESPONSE];

paxdiablo has explained that you're also not invoking sscanf_s correctly. One possible fix here is to switch to calling scanf, passing the max string size and checking for user input

while (scanf(" %3s", well) != 1);

Of, sticking with scanf_s

while (scanf_s(" %s", well, sizeof(well)-1) != 1);
simonc
  • 41,632
  • 12
  • 85
  • 103
  • 1
    Or better yet, don't use strange magic numbers in the code. `#define WORST_CASE (sizeof("Yes"))` ... `char well [WORST_CASE];` – Lundin May 03 '13 at 11:10
  • @Lundin Good point, I've updated my answer as suggested – simonc May 03 '13 at 11:13
  • Many thanks for the advice unfortunately trying the suggested didn't seem to have any affect. – user2346526 May 03 '13 at 16:03
  • @user2346526 There were a couple of errors in my `scanf` suggestion. I've corrected them now; can you try again? – simonc May 03 '13 at 16:17
1

For a start, you don't have enough space in well to store 3 characters and a null terminator.

And scanf_s is secure because it requires (see here) certain format strings like s to have lengths, which you're missing.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
0

allocate 4 bytes to well for a terminating null character.. this solves your issue..

char well[4];
Raghu Srikanth Reddy
  • 2,703
  • 33
  • 29
  • 42
0
scanf_s ("%s",well);

scanf_s requires length for string:

scanf_s ("%s", well, 4);

and 

char well[4];
Sirko
  • 72,589
  • 19
  • 149
  • 183
stinkpot
  • 70
  • 1
  • 9
0

Every character array that stores a string has an additional character \0 that serves as the string terminator.That signifies the end of the string.So "Yes" needs an array of size sizeof(char)*4 to be stored.In your program, only the first 3 characters is stored into well.Hence the strcmp() returns a non-zero number,and hence !strcmp is always 0 due to which the control always jumps to else.The following is the working code:

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

int main()
{
   char well[4];// "Yes" needs sizeof(char)*4, to account for the end `\0`

   printf("Hello, are you well today? (Yes/No) ");
   scanf ("%s",well);      //Use scanf(),scanf_s() works for Uncle Gates only

       if (!strcmp(well,"Yes"))
       printf("Well...glad to hear it, so am I\n");

       else
       printf("Sorry to hear that,umm....can I take your wife out?\n");

system("PAUSE");   //This works for Uncle Gates only (not portable)
return 0;
}
Rüppell's Vulture
  • 3,583
  • 7
  • 35
  • 49
  • I wouldn't use `scanf(%s)` either, you're swapping a non-portable solution (which is actually not too bad if you know you won't be moving off MSVC++) for an unsafe one - see here for why: http://stackoverflow.com/questions/4023895/how-to-read-string-entered-by-user-in-c/4023921#4023921 – paxdiablo May 03 '13 at 11:33
  • @paxdiablo There is always `gets()` to fall back on. – Rüppell's Vulture May 03 '13 at 11:34
  • @paxdiablo `wont' be moving off MS world` is the attitude why I am alien to the world of Linux to this day, and regretting every moment of it. – Rüppell's Vulture May 03 '13 at 11:35
  • I can't tell whether you _meant_ the humour in that response, Vulture. I assume you meant `fgets`, yes? `gets` is every bit as unsafe as `scanf(%s)`. – paxdiablo May 03 '13 at 11:35
  • @paxdiablo What's wrong about `gets()`?Does that have a downside as well? – Rüppell's Vulture May 03 '13 at 11:36
  • Same problem, no way to prevent buffer overflow. Enter a 20 character string into `char buff[4]` and all bets are off. `fgets` allows you to specify a limit. – paxdiablo May 03 '13 at 11:37
  • @paxdiablo Oh,I was only talking about portability.Yes, as for buffer overflow, `fgets()` should be preferred. – Rüppell's Vulture May 03 '13 at 11:38
  • @paxdiablo But the problem of `jumping to else` persists if the OP tries to pass **3** as the second parameter to `fscanf`.That way he will get the string `{'Y','e','\0'}`.It's about how the OP sees strings.He clearly needs to realize that there's an additional character at the end of a string. – Rüppell's Vulture May 03 '13 at 11:42
  • Thanks for providing the detailed break down to the code above, running the above code however causes a fatal error when compiled which despite a fair amount of research seems to only be fixable by removing the whole if statement! – user2346526 May 03 '13 at 16:06
0

allocate 4 bytes to well,then memset to NULL,then give a try

char well[4];
memset(well,0,4)
ytaoWang
  • 1
  • 1