brenns10 / lsh

Simple shell implementation. Tutorial here ->
http://brennan.io/2015/01/16/write-a-shell-in-c/
The Unlicense
1.44k stars 341 forks source link

Handling of EOF in lsh_read_line() #25

Open RobSim0116 opened 1 month ago

RobSim0116 commented 1 month ago

Hi :) I just wanted to contribute to your excellent tutorial by (I think) improving the error handling in lsh_read_line(). Unless I'm mistaken, I don't think it currently handles the case when EOF actually indicates a read error and not a "successful" EOF/an EOF proper. My own code for the while loop looks more or less like this at the moment (you can see I'm (a) checking for the meaning of the returned EOF and (b) ensuring the buffer is freed before exiting):

// rest of code

while (1) {
        c = getc(stdin);

        if (c == '\n') {
            rl_buf[pos] = '\0';
            return rl_buf;
        } else if (c == EOF && ferror(stdin)) {
            fprintf(stderr, "ERROR: read error (%d)\n", errno);
            free(rl_buf);
            exit(EXIT_FAILURE);
        } else if (c == EOF && feof(stdin)){
            free(rl_buf);
            exit(EXIT_SUCCESS);
        }

        rl_buf[pos] = c;
        pos++;

        // rest of code
}

What do you think? Have I made a mistake in my reasoning?

brenns10 commented 2 weeks ago

You're definitely correct! If you'd like to submit a pull request with that change, I'd be happy to merge it. One slight change I would suggest is rather than the fprintf() call you have, you can use the perror(3) function:

perror("getchar");

This will automatically lookup what the error was (using the errno variable and then strerror(3)), and it will print that message to stderr, prefixed by whatever you put in as the first string argument. So, for example, you might get something like this printed to stderr:

getchar: Bad file descriptor