Hacker News Reader

Comment by tlrobinson | Hacker News Reader

Nice hack. Ideally it would just be a feature of the shell. At least it does a isatty check though.

Is there a reason you allocate a bunch of memory and copy the strings rather than just doing multiple writes?

    (*lol_write)(fd, STDERR_COLOR, STDERR_COLOR_SIZE);
    (*lol_write)(fd, buf, count);
    (*lol_write)(fd, COL_RESET, COL_RESET_SIZE);

View on HN

Or use writev for that matter.

Note that using ordinary write multiple times has atomicity implications - if you have multiple processes writing to STDERR, you might find their output interleaved in unfortunate ways. Using a single write with a temporary buffer or writev avoids this (provided you're not writing more than PIPE_BUF bytes total)

Indeed: https://github.com/sickill/stderred/pull/1

I agree that writev is a better approach. But your patch doesn't look thread-safe because it uses that static iovec struct.

You are right, and there are other issues: - I don't return the proper return value (should not return the number of bytes from the color strings) - I don't handle the case where writev() returns a number of bytes between 0 and count (e.g. You are writing 10Mb of data, a signal arrives, and you only wrote 5Mb -- the color thing will get all confused)

The reason would be interleaving if more than one process is writing to stderr.

I was more bothered by the use of alloca(). Even on a 32-bit system with a 32-bit size_t, you can blow your stack to hell with a single call to this modified write(). Having to do a full malloc() here would be pretty lame, but as others suggested, writev() should do nicely.

The other nice property of using writev() is that you don't have to bother with the dlopen/dlsym crap. Can just call writev() directly from the overridden write() in either case.

Of course, that doesn't catch people writing to stderr using writev() directly, but I think that's ok. (The current impl doesn't catch that case anyway.)

(edit: pull request submitted! https://github.com/sickill/stderred/pull/6)

Like bdolan said, it would have atomicity implications this way. I agree it would be lighter on memory but in some cases output might break if many processes were writing to stderr at the same time.

Reply on news.ycombinator.com