Reading from stdin using fgets()

Question

I'm new to C programming and I'm currently trying to read a line from stdin using fgets(), but I'm having trouble with memory allocation since I'm using a char* to point to the string I want to read. When I execute the file it reports a segmentation fault.

This is the function I'm using:

char *read_line(char *line){
    printf("%s",PROMPT);
    line = (char*)malloc(sizeof(char)*500);
    fgets(line,sizeof(line),stdin);
    printf("%s","pasa el fgets");
    return line;
}

And my main:

void main(){
    char line0;
    char *line=&line0;
    while(read_line(line)){
        execute_line(line);
    }
}

char line0; char *line=&line0; Now line points to a char buffer of length 1. If your line is more than 1 byte in length, you'll overrun the buffer. Instead of just jamming code in until it compiles, try to actually understand what it is doing. I'd recommend a C tutorial, book, or course. — Jonathon Reinhart
sizeof(line) is the size of a pointer, not the size of the allocate memory. — chux - Reinstate Monica
1) fgets(line,sizeof(line),stdin); --> fgets(line, 500, stdin); 2) while(read_line(line)){ --> while(line = read_line(line)){ //need free(line); and break loop — BLUEPIXY

undur_gongor undur_gongor · Accepted Answer · 2016-11-03T22:08:00

The main mistake is to pass the pointer line to the function read_line (by value) and try to modify it in that function.

read_line allocates the memory and actually creates the pointer value. So it should be able to change the value of line in main:

char *read_line(char **line){
    ...
    *line = malloc(500);
    fgets(*line, 500, stdin);
    ...
    return *line;
}

int main(void) {
    char *line;
    while(read_line(&line)){
        ...
    }

}

Or, you use the return value of read_line in order to modify main's line. In that case you don't need the parameter at all:

char *read_line(void) {
    char *line;
    ...
    line = malloc(500);
    fgets(line, 500, stdin);
    ... 
    return line;
}

int main(void) {
    char *line;
    while(line = read_line()){
        ...
    }
}

Additional errors (pointed out by Jonathon Reinhart) and remarks:

sizeof does not "work" for pointers (array decayed to pointers).
You malloc many strings line but you do not free them.
sizeof(char) is always 1.
Some people (me too) think that casting the result of malloc should be avoided.

Reading from stdin using fgets()

1 Answers