convert read_packed_refs to use strbuf
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest3 patches 1 replyIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | read_packed_refs: use a strbuf for reading linesrefs.c | +11 | 0 replies | |
| 2/3 | read_packed_refs: pass strbuf to parse_ref_linerefs.c | +15 | 0 replies | |
| 3/3 | read_packed_refs: use skip_prefix instead of static arrayrefs.c | +2 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 0/3] convert read_packed_refs to use strbuf
Quoted 12 lines · Jeff King
Quoted 3 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)) { ... }
Quoted 3 lines
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)".
Quoted 5 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