Segmentation fault while operating on user input string

Question

Hi Im new to coding in C and I can't seem to find the problem which causes a segfault after the while loop starts.

int main() {
    char option;
    char nickname;

    printf("Welcome to our chat program. Please select mode (Send,Nickname,Exit): ");

    while (gets(option)) {

        if(!strncmp(option, "Exit", 4)) {
            break;
        }

        if(!strncmp(option, "Nickname", 8)){
            set_nickname(nickname);
        }

        if(!strncmp(option, "Send", 4)){
            if(!nickname){
            nickname = "Anonymous";
            printf("Your nickname was set to Anonymous!");
            }
            send_message(nickname);
        }
    }

You should compile with some warning flags: -W -Wall by example. Then you'll see your errors. — nouney
– nouney, Commented Jan 13, 2016 at 13:24
It looks like you're only holding one char for option and nickname whilst you're comparing them with multiple character strings. That might be the problem. — birdoftheday
– birdoftheday, Commented Jan 13, 2016 at 13:26
Compile with all warnings & debug info (gcc -Wall -Wextra -g). Use the debugger (gdb) & valgrind. BTW gets is obsolete and should never be used. Use fgets, getline, readline — Basile Starynkevitch
– Basile Starynkevitch, Commented Jan 13, 2016 at 13:31

Sourav Ghosh · Accepted Answer · 2016-01-13 13:32:08Z

3

There are many issues with the code. Let's discuss them one by one.

First of all, a char is not sufficient to hold a string, you need either
- an array
- a pointer with proper (dynamic) memory allocation.
Using the first approach, you need to change
```
char option;
char nickname;
```
to
```
#define SIZ 64

char option[SIZ] = {0};
char nicknamep[SIZ] = {0};
```
Then, instead of using gets(), you should be using fgets() to avoid the possibility of buffer overflow. Something like
```
fgets(option, SIZ, stdin);
```
will do.
That said, once option becomes an array, you cannot assign to it. You need to use strcpy() to copy the content into the array. For example,
```
strcpy(nickname, "Anonymous");
```

edited Jan 13, 2016 at 13:32

answered Jan 13, 2016 at 13:27

Sourav Ghosh

135k17 gold badges192 silver badges271 bronze badges

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

Comments

Danny_ds · Accepted Answer · 2016-01-13 13:27:26Z

1

There is no memory allocated for the strings.

You need someting like this:

char option[50];
char nickname[50];

and use strcpy() here:

nickname = "Anonymous";

and check the string with strlen() for example.

if(!nickname){

It is also safer to use fgets() with a length parameter to prevent buffer overflow.

while (gets(option)) {

edited Jan 13, 2016 at 13:27

answered Jan 13, 2016 at 13:21

Danny_ds

11.4k1 gold badge26 silver badges48 bronze badges

Comments

LoztInSpace · Accepted Answer · 2016-01-13 13:22:30Z

0

option needs to be char[???]

Not sure about nickname but it also needs memory allocated and/or initialisation. How do you set it? What value do you use?

You'll also need to check for strlen(nickname)>0 rather than !nickname which will test for a null pointer, not an empty string.

answered Jan 13, 2016 at 13:22

LoztInSpace

5,6971 gold badge17 silver badges29 bronze badges

Comments

Wouter Verhelst · Accepted Answer · 2016-01-13 13:39:46Z

Never ever use gets; it cannot be used correctly. Instead, use fgets, with stdin as the third parameter.

You will notice that fgets takes the length of the storage you want to assign as its second parameter. The lack of this parameter in the gets function is why it cannot be used correctly. You should pass it the value 1 in your case, and pass it the address of your char variable. fgets will then read one character into your variable.

If you want to read more, you need to start by allocating more storage for the data you'll read. For instance, you could allocate space for 80 characters:

char string[80];

You can now use fgets with string as the first character, 80 as the second, and stdin as the third. fgets will then read up to 80 characters from stdin.

If you don't want to store that number in two locations in your source code, you can use the sizeof operator instead, or use a macro to predefine the size of your buffer.

Collectives™ on Stack Overflow

Segmentation fault while operating on user input string

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related