On Wed, Aug 03, 2016 at 06:42:16PM +0200, larsxschneider@xxxxxxxxx wrote:
> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>
> packet_flush() would die in case of a write error even though for some callers
> an error would be acceptable. Add packet_flush_gentle() which writes a pkt-line
> flush packet and returns `0` for success and `1` for failure.
Our normal convention would be "0" for success, "-1" for failure.
I see write_or_whine_pipe(), which you use here, has a bizarre "0 for
failure, 1 for success", but that nobody actually checks it.
I actually think you probably don't want to use write_or_whine_pipe()
here. It does two things:
1. It writes to stderr unconditionally. But if you are doing a
"gently" form, then you probably don't want unconditional errors.
Since the point of not dying is that you could presumably recover
in some way, or do some other more intelligent action.
The existing callers of write_or_whine_pipe() are all in the trace
code. Their use is not "let's handle an error", but "we _would_ die
except that this is low-priority debugging code that should not
interrupt the normal flow". So there it at least makes sense to
unconditionally complain to stderr, but not to die().
For your series, I don't think that is true (and especially for
most potential callers of a generic "gently flush the packet"
function).
2. It calls check_pipe(), which will turn EPIPE into death-by-SIGPIPE
(in case you had for some reason ignored SIGPIPE).
But I think that's the opposite of what you want. You know you're
writing to a pipe, and I would think EPIPE is the most common
reason that your writes would fail (i.e., the helper unexpectedly
died while you were writing to it).
So you would want to explicitly ignore SIGPIPE while talking to the
helper, and then handle EPIPE just as any other error.
Thinking about (2), I'd go so far as to say that the trace actually
should just be using:
if (write_in_full(...) < 0)
warning("unable to write trace to ...: %s", strerror(errno));
and we should get rid of write_or_whine_pipe entirely.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html