Re: [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK

From: Jeff King <peff@peff.net>
Date: Jun 27, 2016, 14:36 UTC
Message-ID: <20160627143648.GA2618@sigill.intra.peff.net>
In-Reply-To: <xmqqoa6mdbu3.fsf@gitster.mtv.corp.google.com>

On Mon, Jun 27, 2016 at 06:02:12AM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I also wondered how we managed to miss such an obvious point in review
> > of the original patch. Sadly, we _did_ notice it[1] but it looks like we
> > never fixed the problem. That is even more disturbing.
> 
> Yes indeed.
> 
> I try to pay attention to "this is broken because..."  comments in
> discussions to make a note in my copy of "What's cooking" report for
> a problematic topic, as that is where I work from when merging
> topics down, but apparently that procedure failed work in this case.
> There needs a stronger mechanism to stop a known-buggy patch from
> going thru, but I am not sure offhand what that should be.

I was the one who saw the bug. I could have followed the series more closely to make sure my concern was addressed. Or possibly pointed out the bug more prominently than an in a "PS" as part of the discussion.

I think part of the problem was that this particular series was large-ish and involved a lot of re-rolls, and I got sick of looking at it. I dunno.

It's also true that our error rate will never be 0%. So some bugs will always slip through, some review comments will be forgotten, etc. Eric did find and fix the bug just now, so the "many eyes" theory did work here eventually.

-Peff