simple input. program crashes

Question

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

int main(){        
    char *command="0";

    do {   
      printf("[A]dd, [P]rint, [Q]uit\n");
      scanf("%s", command);

    while (strcmp(command, "a") != 0 && strcmp(command, "A") != 0 && strcmp(command, "p") != 0 && strcmp(command, "P") != 0){
        printf("Invalid input. Please enter one of the commands listed above.\n");
        scanf("%s", command);
    }       

       if (strcmp(command, "a") == 0 || strcmp(command, "A") == 0){
           printf("You selected add.\n");
       }
       else if (strcmp(command, "p") == 0 || strcmp(command, "P") == 0){
           printf("You selected print.\n");
       }
    }while (strcmp(command, "q") != 0 && strcmp(command, "Q")!= 0);
    return 0;
}

I want the program to take in a letter from the user from one of the specified commands printed in the beginning. I want the program to exit if they enter q or Q. Took me a while simply to figure out how to do comparisons with strings for the loops and ifs. now when i run the program it crashes though. Looking for insight as to why its crashing.

Where does it crash? When you attach a debugger, what is the state of the program when it crashes? Does the actual state match your expected state? — James McNellis
– James McNellis, Commented Mar 4, 2012 at 22:35
You can't change the contents of command: it points to a string literal. Try an array instead: char command[] = "0";. and be sure to limit the length of the string read with the scanf: scanf("%1s") — pmg
– pmg, Commented Mar 4, 2012 at 22:35
scanf is trying to write N bytes to the address pointed by command, which is a string-literal (read-only). You need to allocate enough memory to store these N bytes, or declare a fixed-length array of chars. Example: char command[255]; scanf("%254s", command); — jweyrich
– jweyrich, Commented Mar 4, 2012 at 22:37

alexis · Accepted Answer · 2012-03-04 22:41:09Z

1

Yes, make your buffer command into a regular array, and make it larger:

char command[256];

No need to initialize it, scanf will take care of that. Also, it won't affect the crashing but if you're only checking one letter, you can do it like this:

if command[0] != 'a' && command[0] != 'A' (etc.)

Note the single quotes: This is a character comparison.

answered Mar 4, 2012 at 22:41

alexis

50.4k18 gold badges108 silver badges173 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

ouah · Accepted Answer · 2012-03-04 22:35:12Z

0

char *command="0";
scanf("%s", command);

you are writing to a string literal (of only 2 bytes) and string literals are not required to be modifiable.

Use something like this instead:

char command[256];
scanf("%s", command);

or better use fgets + sscanf to avoid potential buffer overflows.

answered Mar 4, 2012 at 22:35

ouah

147k16 gold badges287 silver badges338 bronze badges

Comments

Dietrich Epp · Accepted Answer · 2012-03-04 22:36:58Z

0

This is a pointer to read-only memory:

char *command="0";

Which means that this is wrong:

scanf("%s", command);

The "quick hack", which is the bad way to do it, is change the definition of command so it gets some read-write memory instead of read-only memory:

char command[256] = "0";

But this still puts you in grave danger of buffer overflow. Don't use scanf.

The "better way" is to use fgets instead of scanf:

char command[256];
char *p;
p = fgets(command, sizeof(command), stdin);
if (!p) { handle error or EOF }
/* Don't forget that 'command' will probably have '\n' at the end,
   you will have to strip it off */

answered Mar 4, 2012 at 22:36

Dietrich Epp

216k39 gold badges366 silver badges426 bronze badges

4 Comments

Tristan Pearce Over a year ago

fgets and sccanf better in this case?

jweyrich Over a year ago

The format string passed to scanf allows limiting the string size. Example: "%255s"

Dietrich Epp Over a year ago

@jweyrich: That's nice, but there are still 99 other reasons not to use scanf.

Dietrich Epp Over a year ago

@TristanPearce: Avoid sscanf, scanf, fscanf, vfscanf, vscanf, and vsscanf. Using them correctly is too tricky.

Ernest Friedman-Hill · Accepted Answer · 2012-03-04 22:37:10Z

0

Well, the first thing that pops out at me is that you're using scanf to read data into read-only memory -- i.e., a string constant. What happens when you try this is undefined, but a program crash is likely. You need to allocate some read/write memory for command; declare it as, for example,

char command[10];

answered Mar 4, 2012 at 22:37

Ernest Friedman-Hill

81.9k11 gold badges158 silver badges192 bronze badges

Collectives™ on Stack Overflow

simple input. program crashes

4 Answers 4

Comments

Comments

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related