From: Ben Peart Date: Thu, 20 Apr 2017 17:27:44 GMT Subject: Re: [PATCH v5 3/8] convert: Split start_multi_file_filter into two separate functions Message-ID: In-Reply-To: <20170411200520.oivytvlzkdu7bfh5@sigill.intra.peff.net> On 4/11/2017 4:05 PM, Jeff King wrote: > On Tue, Apr 11, 2017 at 10:01:02PM +0200, Lars Schneider wrote: > >>> If you initialize errno to 0 right before a syscall, then yes, you can >>> trust it without checking the return value of the syscall. I wouldn't >>> trust it before calling more complicated functions, though. Not even >>> xwrite(), which may see EINTR and keep going (which is OK for checking >>> for EPIPE, but not checking generally for errno values). >> Should we remove all the errno checks here as we don't have any direct >> "write" etc syscalls anyways then? > Yeah, you should be trusting the return value from the various > sub-functions. Usually you'd check "errno == EPIPE" only when you saw an > error return but you want to _ignore_ EPIPE. This is what > filter_buffer_or_fd() is doing. > > But the code here is the opposite case. It definitely wants to treat > EPIPE as an error. But that should be happening already because any > EPIPE we get would come with an error-return from one of the > packet_write() functions. > > So I would say that "err || errno == EPIPE" here can just become "err", > and ditto in apply_multi_file_filter(). I'll update it this way in the next spin of the patch series. > -Peff