[PATCH 0/3] convert read_packed_refs to use strbuf

From: Jeff King <peff@peff.net>
Date: Dec 10, 2014, 10:39 UTC
Message-ID: <20141210103907.GA22186@peff.net>
In-Reply-To: <20141210095319.GA9099@peff.net>

On Wed, Dec 10, 2014 at 04:53:19AM -0500, Jeff King wrote:

Show 15 quoted lines

> > Clarification: for-each-ref ignores the ref when the full line read
> > from packed-refs hits length 1024 (not when the refname itself hits
> > length 1024).
> 
> Yes, the problem is in read_packed_refs:
> 
>     char refline[PATH_MAX];
>     ...
>     while (fgets(refline, sizeof(refline), f)) {
>         ...
>     }
> 
> This could be trivially converted to strbuf_getwholeline, but I am not
> sure what else would break, or whether such a system would actually be
> _usable_ with such long refs (e.g., would it break the first time you

I accidentally cut off the next line, but it was something like "...first time you actually tried writing to the ref)".

Show 5 quoted lines

> Using fgets like this does shear lines, though. The next fgets call will
> see the second half of the line. I think we are saved from doing
> anything stupid by parse_ref_line, but it is mostly luck. So perhaps for
> that reason the trivial conversion to strbuf is worth it, even if it
> doesn't help any practical cases.

Here's a patch to do that. It still doesn't let you create long refs on OS X, as we get caught up in the PATH_MAX found in git_path() and friends. Still, I think it's a step in the right direction, and it fixes the shearing issue.

Patches 2 and 3 are just follow-on cleanups.

  [1/3]: read_packed_refs: use a strbuf for reading lines
  [2/3]: read_packed_refs: pass strbuf to parse_ref_line
  [3/3]: read_packed_refs: use skip_prefix instead of static array

I checked, and this miraculously does not conflict with any of the refs work in pu. :)

-Peff