0

gcc compiles perfectly well, but as soon as scanf accepts a string it seg faults. I'm sort of at a loss. Here's the code.

char *line[256];


void *mainThread()
{
        while (*line != "quit") {
                scanf("%s", *line);
                printf("%s", *line);
        }

        return;
}

Is there something about scanf I'm not understanding here?

Ormannishe
  • 55
  • 8

3 Answers3

5

First, you are allocating an array of pointers to characters, not an array of char:

char *line[256]; /* allocates 256 pointers to a character - 
                    (pointers are initialized to NULL) */

You need to allocate an array of characters instead:

char line[256]; /* allocates 256 characters */

Second, you need to use strcmp to compare the strings - with !=, you are comparing the pointer (address) stored in line[0] (which is the same as *line) with a pointer to the string literal "quit", and they are always different.

You can use the following sscce as a starting point:

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

char line[256];

int main()
{
    while (strcmp(line, "quit") != 0) {
       scanf("%s", line);
       printf("%s", line);
    }

    return 0;
}

Some additional notes:

  • See @Joachims answer for an explanation of the actual cause of the segmentation fault.
  • You are declaring your function to return a void* pointer, but you are not returning anything (using return with no argument). You should then simply declare it as void.
  • Do not use scanf() to read input, since it might read more characters than you have allocated which leads to buffer overflows. Use fgets() instead. See also Disadvantages of scanf.
  • Always compile with all warnings enabled, and take them serious - e.g. if you are using gcc, compile with -Wall -pedantic.
Community
  • 1
  • 1
Andreas Fester
  • 36,091
  • 7
  • 95
  • 123
  • 1
    You might also want to mention that [this program deals rather horribly with larger inputs](http://stackoverflow.com/q/1621394/274261). – ArjunShankar Mar 05 '14 at 07:55
  • Thanks! strcmp was an obvious mistake, silly me. And i don't want to return anything, this function returns a void* pointer because I'm using pthreads. – Ormannishe Mar 05 '14 at 18:58
2

When declaring global variables, they are initialized to zero. And as you are declaring an array of pointers to char (instead of an array of char as I think you really intended) you have an array of 256 NULL pointers.

Using the dereference operator * on an array is the same as doing e.g. array[0], which means that as argument to both scanf and printf you are passing line[0], which as explained above, is a NULL pointer. Dereferencing NULL pointers, like scanf and printf will do, is a case of undefined behavior, one that almost always leads to a crash.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

It is compiling because your program is syntactically correct. However, it has serious semantic errors which show up as program crash due to segfault. The global array line is initialized to zero, as any global variable. Since, line is an array of pointers (not an array of characters which is intended), the zeros are interpreted as NULL, the null pointer. *line is same as line[0] and the string literal "quit" evaluates to a pointer to its first element. Therefore the while condition is the same as

while(NULL != "quit") // always true

Next, scanf("%s", *line); tries to write the input string into the buffer pointed to by line[0] which is NULL - a value which is unequal to the address of any memory location. This will result in segfault and cause the program to crash.

There are other mistakes in your code snippet. Let's take them one by one.

char *line[256];

The above statement defines an array line of 256 pointers to characters, i.e., its
type is char *[256]. What you need is an array of characters -

char line[256];

You can't compare array in C. What you can do is compare them element-by-element. For strings, you should use the standard library function strcmp. Also, please note that the %s conversion specifier in the format string of scanf reads a string from stdin and writes it into the buffer pointed to by the next argument. It puts a terminating null byte at the end but it does not check for buffer overrun if you input a string too large for the buffer to hold. This would lead to undefined behaviour and most likely segfault due to illegal memory access. You should guard against buffer overrun by specifying maximum field width in the format string.

void *mainThread();

The above function declaration means that mainThread is function which returns a pointer of void * type and take an unspecified but fixed number and type of arguments because empty parentheses mean no information about the parameter list is provided. You should write void in the parameter list to mean that the function takes no arguments. Also note that the empty return statement in your function would cause undefined behaviour if the return value of the function is used because you are not returning anything and would be using the garbage value in the return address of the function instead. Assuming that you want to return the string, it should be defined as -

char line[256];

char *mainThread(void) {
    while(strcmp(line, "quit") != 0) {
        scanf("%255s", line);    // -1 for the terminating null byte
        printf("%s", line);
    }
    return line;
}
ajay
  • 9,402
  • 8
  • 44
  • 71