Re: [PATCH 6/7] worktree: check the result of read_in_full()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 25, 2017 at 03:14:26PM -0700, Jonathan Nieder wrote:

> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 2f4a4ef9cd..87b3d70b0b 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -59,7 +59,11 @@ static int prune_worktree(const char *id, struct strbuf *reason)
> >  	}
> >  	len = xsize_t(st.st_size);
> >  	path = xmallocz(len);
> > -	read_in_full(fd, path, len);
> > +	if (read_in_full(fd, path, len) != len) {
> > +		strbuf_addf(reason, _("Removing worktrees/%s: gitdir read did not match stat (%s)"),
> > +			    id, strerror(errno));
> 
> I'm a little confused.  The 'if' condition checks for a read error but
> the message says something about 'stat'.
> 
> If we're trying to double-check the 'stat' result, shouldn't we read
> all the way to EOF in case the file got longer?

If you wanted to really double-check the stat result, yes you'd want to
make sure there aren't extra bytes either. But really we're just making
sure we were able to read _enough_ bytes.

To be honest, I'm tempted to rip out the whole thing and replace it with
strbuf_read_file(), which seems more straightforward.

The function does seem to rely on the stat() to get the mtime, so we'd
have to leave that part in.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux