Re: [PATCH 1/7] files-backend: prefer "0" for write_in_full() error check

From: Jeff King <peff@peff.net>
Date: Sep 25, 2017, 23:13 UTC
Message-ID: <20170925231308.hpk7xrelqyqy6clg@sigill.intra.peff.net>
In-Reply-To: <20170925215927.GZ27425@aiede.mtv.corp.google.com>

On Mon, Sep 25, 2017 at 02:59:27PM -0700, Jonathan Nieder wrote:

Show 5 quoted lines

> > But during the conflict resolution in c50424a6f0 (Merge
> > branch 'jk/write-in-full-fix', 2017-09-25), this morphed
> > into
> [...]
> Good eyes.  Thanks.

Sort of. :) I usually continually rebase my topics until they end up empty. So I notice bits like this either during conflict resolution, or when the topic is left unexpectedly non-empty.

It's not foolproof, though. Sometimes rebasing a topic on itself is so painful that I "rebase --skip" liberally in the early parts, and would miss any merge funniness.

Show 11 quoted lines

> By the way, the description from that merge commit
> 
>     Many codepaths did not diagnose write failures correctly when disks
>     go full, due to their misuse of write_in_full() helper function,
>     which have been corrected.
> 
> does not look accurate to me.  At least the "Many codepaths" part.
> All of those changes except for three should be no-ops.  The scariest
> one is the 'long ret = write_in_full(fd, buf, size)' in notes-merge.c,
> which is about overflowing a "long" on Windows instead of error
> handling.

The ones in config.c are definitely wrong, too. Though I'd agree that "many" might be overstating it.

-Peff