Check that 'fifo' is really a FIFO

[cdlscpmv]

Fixes the bug that if 'fifo' is a regular file then luastatus hangs in a loop.

[shdown]

Good idea, but the current implementation contains a time-of-check/time-of-use race condition: although very unlikely, the file may be overwritten between stat() and open() calls. You should probably use fstat() after open().

Also, I don’t quite like the “magic number” approach to error reporting. Could you just set errno to EINVAL if w->fifo is not actually a FIFO? :)

[cdlscpmv]

Also, I don’t quite like the “magic number” approach to error reporting. Could you just set errno to EINVAL if w->fifo is not actually a FIFO? :)

But should I check errno before printing an error? Otherwise, for EINVAL it will just print "Invalid argument", which doesn't tell the user much.

[shdown]

But should I check errno before printing an error? Otherwise, for EINVAL it will just print "Invalid argument", which doesn't tell the user much.

Well, I guess no. It would print ls_wakeup_fifo_open: <path>: Invalid argument, which is probably OK.

Another approach is to use negative errno values for our custom errors:

// this enum is an implementation detail and is not exported
typedef enum {
    ERR_HIGH_FD = 1,
    ERR_NOT_A_FIFO,
} WakeupFifoError;

// but this function _is_ exported
const char *
ls_wakeup_fifo_strerror(int errnum)
{
    switch ((WakeupFifoError) -errnum) {
    case ERR_HIGH_FD:
        return "File descriptor too high to use with select()";
    case ERR_NOT_A_FIFO:
        return "Not a FIFO";
    }
    LS_UNREACHABLE();
}

// change /ls_wakeup_fifo_open()/ to set /errno/ to /-ERR_HIGH_FD/ and /-ERR_NOT_A_FIFO/ in appropriate situations

And to get the error string, the user has to do

errno > 0 ? ls_strerror_onstack(errno) : ls_wakeup_fifo_strerror(errno)

which is not too verbose, I guess.

[cdlscpmv]

Take a look at the code. Since there is only one non-standard error in our case, I think it is not worth it to create a custom error type for it. I don't think we need to care about ERR_HIGH_FD, because it seems unrealistic that it may get triggered in an ordinary usage of the program. Besides, EMFILE describes the error pretty well.

[shdown]

Take a look at the code.

Nice, but I don’t like the code duplication introduced (errno == -EINVAL ? "The file is not a FIFO" : ls_strerror_onstack(errno)).

[cdlscpmv]

How about modifying ls_strerror_r() to accept negative error numbers? We may introduce a global errno table with negative values and use it for all sorts of non-standard errors.

[shdown]

Merged, thanks!

How about modifying ls_strerror_r() to accept negative error numbers? We may introduce a global errno table with negative values and use it for all sorts of non-standard errors.

No. The less complex, the better.