@matheusbsbs / matheus-bagliano / commits / e5e73ff20b

for_each_reflog_ent_reverse: fix newlines on block boundaries

When we read a reflog file in reverse, we read whole chunks of BUFSIZ bytes, then loop over the buffer, parsing any lines we find. We find the beginning of each line by looking for the newline from the previous line. If we don't find one, we know that we are either at the beginning of the file, or that we have to read another block. In the latter case, we stuff away what we have into a strbuf, read another block, and continue our parse. But we missed one case here. If we did find a newline, and it is at the beginning of the block, we must also stuff that newline into the strbuf, as it belongs to the block we are about to read. The minimal fix here would be to add this special case to the conditional that checks whether we found a newline. But we can make the flow a little clearer by rearranging a bit: we first handle lines that we are going to show, and then at the end of each loop, stuff away any leftovers if necessary. That lets us fold this special-case in with the more common "we ended in the middle of a line" case. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 4, 2014 at 20:28 UTC e5e73ff20b5a750b9622438fd877f4e4a4c3519b

2 files changed +66 -11

refs.c

+36 -11

@@ -3089,24 +3089,49 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void

3089

3090 bp = find_beginning_of_line(buf, scanp);

3091

3092 - if (*bp != '\n') {

3093 - strbuf_splice(&sb, 0, 0, buf, endp - buf);

3094 - if (pos)

3095 - break; /* need to fill another block */

3096 - scanp = buf - 1; /* leave loop */

3097 - } else {

3092 + if (*bp == '\n') {

3093 /*

3099 - * (bp + 1) thru endp is the beginning of the

3100 - * current line we have in sb

3094 + * The newline is the end of the previous line,

3095 + * so we know we have complete line starting

3096 + * at (bp + 1). Prefix it onto any prior data

3097 + * we collected for the line and process it.

3098 */

3099 strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1));

3100 scanp = bp;

3101 endp = bp + 1;

3102 + ret = show_one_reflog_ent(&sb, fn, cb_data);

3103 + strbuf_reset(&sb);

3104 + if (ret)

3105 + break;

3106 + } else if (!pos) {

3107 + /*

3108 + * We are at the start of the buffer, and the

3109 + * start of the file; there is no previous

3110 + * line, and we have everything for this one.

3111 + * Process it, and we can end the loop.

3112 + */

3113 + strbuf_splice(&sb, 0, 0, buf, endp - buf);

3114 + ret = show_one_reflog_ent(&sb, fn, cb_data);

3115 + strbuf_reset(&sb);

3116 + break;

3117 }

3106 - ret = show_one_reflog_ent(&sb, fn, cb_data);

3107 - strbuf_reset(&sb);

3108 - if (ret)

3118 +

3119 + if (bp == buf) {

3120 + /*

3121 + * We are at the start of the buffer, and there

3122 + * is more file to read backwards. Which means

3123 + * we are in the middle of a line. Note that we

3124 + * may get here even if *bp was a newline; that

3125 + * just means we are at the exact end of the

3126 + * previous line, rather than some spot in the

3127 + * middle.

3128 + *

3129 + * Save away what we have to be combined with

3130 + * the data from the next read.

3131 + */

3132 + strbuf_splice(&sb, 0, 0, buf, endp - buf);

3133 break;

3134 + }

3135 }

3136

3137 }

t/t1410-reflog.sh

+30

@@ -245,4 +245,34 @@ test_expect_success 'gc.reflogexpire=false' '

245

246 '

247

248 +# Triggering the bug detected by this test requires a newline to fall

249 +# exactly BUFSIZ-1 bytes from the end of the file. We don't know

250 +# what that value is, since it's platform dependent. However, if

251 +# we choose some value N, we also catch any D which divides N evenly

252 +# (since we will read backwards in chunks of D). So we choose 8K,

253 +# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).

254 +#

255 +# Each line is 114 characters, so we need 75 to still have a few before the

256 +# last 8K. The 89-character padding on the final entry lines up our

257 +# newline exactly.

258 +test_expect_success 'parsing reverse reflogs at BUFSIZ boundaries' '

259 + git checkout -b reflogskip &&

260 + z38=00000000000000000000000000000000000000 &&

261 + ident="abc <xyz> 0000000001 +0000" &&

262 + for i in $(test_seq 1 75); do

263 + printf "$z38%02d $z38%02d %s\t" $i $(($i+1)) "$ident" &&

264 + if test $i = 75; then

265 + for j in $(test_seq 1 89); do

266 + printf X

267 + done

268 + else

269 + printf X

270 + fi &&

271 + printf "\n"

272 + done >.git/logs/refs/heads/reflogskip &&

273 + git rev-parse reflogskip@{73} >actual &&

274 + echo ${z38}03 >expect &&

275 + test_cmp expect actual

276 +'

277 +

278 test_done