Re: [PATCH] reopen_tempfile(): truncate opened file

[Jeff King <peff@peff.net>]

On Wed, Sep 05, 2018 at 05:39:19PM +0200, Duy Nguyen wrote:

> On Wed, Sep 5, 2018 at 5:35 PM Jeff King <peff@peff.net> wrote:
> > > > +       after=$(wc -c <.git/index) &&
> > > > +
> > > > +       # double check that the index shrank
> > > > +       test $before -gt $after &&
> > > > +
> > > > +       # and that our index was not corrupted
> > > > +       git fsck
> > >
> > > If the index is not shrunk, we parse remaining rubbish as extensions.
> > > If by chance the rubbish extension name is in uppercase, then we
> > > ignore (and not flag it as error). But then the chances of the next 4
> > > bytes being the "right" extension size is so small that we would end
> > > up flagging it as bad extension anyway. So it's good. But if you want
> > > to be even stricter (not necessary in my opinion), make sure that
> > > stderr is empty.
> >
> > In this case, the size difference is only a few bytes, so the rubbish
> > actually ends up in the trailing sha1. The reason I use git-fsck here is
> > that it actually verifies the whole sha1 (since normal index reads no
> > longer do). In fact, a normal index read won't show any problem for this
> > case (since it is _only_ the trailing sha1 which is junk, and we no
> > longer verify it on every read).
> >
> > In the original sparse-dev case, the size of the rubbish is much larger
> > (because we deleted a lot more entries), and we do interpret it as a
> > bogus extension. But it also triggers here, because the trailing sha1 is
> > _also_ wrong.
> >
> > So AFAIK this fsck catches everything and yields a non-zero exit in the
> > error case. And it should work for even a single byte of rubbish.
> 
> Yes you're right. I forgot about the trailing hash.

Thanks, I was worried that I was missing something. ;)

Maybe it is worth making that final comment:

  # and that the trailing hash in the index was not corrupted,
  # which should catch even a single byte of cruft
  git fsck

-Peff