@matheusbsbs / matheus-bagliano / commits / 8597ea3afe

commit: record buffer length in cache

Most callsites which use the commit buffer try to use the cached version attached to the commit, rather than re-reading from disk. Unfortunately, that interface provides only a pointer to the NUL-terminated buffer, with no indication of the original length. For the most part, this doesn't matter. People do not put NULs in their commit messages, and the log code is happy to treat it all as a NUL-terminated string. However, some code paths do care. For example, when checking signatures, we want to be very careful that we verify all the bytes to avoid malicious trickery. This patch just adds an optional "size" out-pointer to get_commit_buffer and friends. The existing callers all pass NULL (there did not seem to be any obvious sites where we could avoid an immediate strlen() call, though perhaps with some further refactoring we could). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 10, 2014 at 17:44 UTC 8597ea3afea067b39ba7d4adae7ec6c1ee0e7c91

16 files changed +68 -38

builtin/blame.c

+13 -1

@@ -1998,6 +1998,18 @@ static void append_merge_parents(struct commit_list **tail)

1998 strbuf_release(&line);

1999 }

2000

2001 +/*

2002 + * This isn't as simple as passing sb->buf and sb->len, because we

2003 + * want to transfer ownership of the buffer to the commit (so we

2004 + * must use detach).

2005 + */

2006 +static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)

2007 +{

2008 + size_t len;

2009 + void *buf = strbuf_detach(sb, &len);

2010 + set_commit_buffer(c, buf, len);

2011 +}

2012 +

2013 /*

2014 * Prepare a dummy commit that represents the work tree (or staged) item.

2015 * Note that annotating work tree item never works in the reverse.

@@ -2046,7 +2058,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,

2058 ident, ident, path,

2059 (!contents_from ? path :

2060 (!strcmp(contents_from, "-") ? "standard input" : contents_from)));

2049 - set_commit_buffer(commit, strbuf_detach(&msg, NULL));

2061 + set_commit_buffer_from_strbuf(commit, &msg);

2062

2063 if (!contents_from || strcmp("-", contents_from)) {

2064 struct stat st;

builtin/fast-export.c

+1 -1

@@ -289,7 +289,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)

289 rev->diffopt.output_format = DIFF_FORMAT_CALLBACK;

290

291 parse_commit_or_die(commit);

292 - commit_buffer = get_commit_buffer(commit);

292 + commit_buffer = get_commit_buffer(commit, NULL);

293 author = strstr(commit_buffer, "\nauthor ");

294 if (!author)

295 die ("Could not find author in commit %s",

builtin/fmt-merge-msg.c

+1 -1

@@ -236,7 +236,7 @@ static void record_person(int which, struct string_list *people,

236 const char *field;

237

238 field = (which == 'a') ? "\nauthor " : "\ncommitter ";

239 - buffer = get_commit_buffer(commit);

239 + buffer = get_commit_buffer(commit, NULL);

240 name = strstr(buffer, field);

241 if (!name)

242 return;

builtin/index-pack.c

+1 -1

@@ -774,7 +774,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,

774 }

775 if (obj->type == OBJ_COMMIT) {

776 struct commit *commit = (struct commit *) obj;

777 - if (detach_commit_buffer(commit) != data)

777 + if (detach_commit_buffer(commit, NULL) != data)

778 die("BUG: parse_object_buffer transmogrified our buffer");

779 }

780 obj->flags |= FLAG_CHECKED;

builtin/log.c

+1 -1

@@ -919,7 +919,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,

919 &need_8bit_cte);

920

921 for (i = 0; !need_8bit_cte && i < nr; i++) {

922 - const char *buf = get_commit_buffer(list[i]);

922 + const char *buf = get_commit_buffer(list[i], NULL);

923 if (has_non_ascii(buf))

924 need_8bit_cte = 1;

925 unuse_commit_buffer(list[i], buf);

builtin/rev-list.c

+1 -1

@@ -106,7 +106,7 @@ static void show_commit(struct commit *commit, void *data)

106 else

107 putchar('\n');

108

109 - if (revs->verbose_header && get_cached_commit_buffer(commit)) {

109 + if (revs->verbose_header && get_cached_commit_buffer(commit, NULL)) {

110 struct strbuf buf = STRBUF_INIT;

111 struct pretty_print_context ctx = {0};

112 ctx.abbrev = revs->abbrev;

commit.c

+36 -18

@@ -245,22 +245,31 @@ int unregister_shallow(const unsigned char *sha1)

245 return 0;

246 }

247

248 -define_commit_slab(buffer_slab, void *);

248 +struct commit_buffer {

249 + void *buffer;

250 + unsigned long size;

251 +};

252 +define_commit_slab(buffer_slab, struct commit_buffer);

253 static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);

254

251 -void set_commit_buffer(struct commit *commit, void *buffer)

255 +void set_commit_buffer(struct commit *commit, void *buffer, unsigned long size)

256 {

253 - *buffer_slab_at(&buffer_slab, commit) = buffer;

257 + struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);

258 + v->buffer = buffer;

259 + v->size = size;

260 }

261

256 -const void *get_cached_commit_buffer(const struct commit *commit)

262 +const void *get_cached_commit_buffer(const struct commit *commit, unsigned long *sizep)

263 {

258 - return *buffer_slab_at(&buffer_slab, commit);

264 + struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);

265 + if (sizep)

266 + *sizep = v->size;

267 + return v->buffer;

268 }

269

261 -const void *get_commit_buffer(const struct commit *commit)

270 +const void *get_commit_buffer(const struct commit *commit, unsigned long *sizep)

271 {

263 - const void *ret = get_cached_commit_buffer(commit);

272 + const void *ret = get_cached_commit_buffer(commit, sizep);

273 if (!ret) {

274 enum object_type type;

275 unsigned long size;

@@ -271,29 +280,38 @@ const void *get_commit_buffer(const struct commit *commit)

280 if (type != OBJ_COMMIT)

281 die("expected commit for %s, got %s",

282 sha1_to_hex(commit->object.sha1), typename(type));

283 + if (sizep)

284 + *sizep = size;

285 }

286 return ret;

287 }

288

289 void unuse_commit_buffer(const struct commit *commit, const void *buffer)

290 {

280 - void *cached = *buffer_slab_at(&buffer_slab, commit);

281 - if (cached != buffer)

291 + struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);

292 + if (v->buffer != buffer)

293 free((void *)buffer);

294 }

295

296 void free_commit_buffer(struct commit *commit)

297 {

287 - void **b = buffer_slab_at(&buffer_slab, commit);

288 - free(*b);

289 - *b = NULL;

298 + struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);

299 + free(v->buffer);

300 + v->buffer = NULL;

301 + v->size = 0;

302 }

303

292 -const void *detach_commit_buffer(struct commit *commit)

304 +const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)

305 {

294 - void **b = buffer_slab_at(&buffer_slab, commit);

295 - void *ret = *b;

296 - *b = NULL;

306 + struct commit_buffer *v = buffer_slab_at(&buffer_slab, commit);

307 + void *ret;

308 +

309 + ret = v->buffer;

310 + if (sizep)

311 + *sizep = v->size;

312 +

313 + v->buffer = NULL;

314 + v->size = 0;

315 return ret;

316 }

317

@@ -374,7 +392,7 @@ int parse_commit(struct commit *item)

392 }

393 ret = parse_commit_buffer(item, buffer, size);

394 if (save_commit_buffer && !ret) {

377 - set_commit_buffer(item, buffer);

395 + set_commit_buffer(item, buffer, size);

396 return 0;

397 }

398 free(buffer);

@@ -589,7 +607,7 @@ static void record_author_date(struct author_date_slab *author_date,

607 struct commit *commit)

608 {

609 const char *buf, *line_end, *ident_line;

592 - const char *buffer = get_commit_buffer(commit);

610 + const char *buffer = get_commit_buffer(commit, NULL);

611 struct ident_split ident;

612 char *date_end;

613 unsigned long date;

commit.h

+4 -4

@@ -54,20 +54,20 @@ void parse_commit_or_die(struct commit *item);

54 * Associate an object buffer with the commit. The ownership of the

55 * memory is handed over to the commit, and must be free()-able.

56 */

57 -void set_commit_buffer(struct commit *, void *buffer);

57 +void set_commit_buffer(struct commit *, void *buffer, unsigned long size);

59 /*

60 * Get any cached object buffer associated with the commit. Returns NULL

61 * if none. The resulting memory should not be freed.

62 */

63 -const void *get_cached_commit_buffer(const struct commit *);

63 +const void *get_cached_commit_buffer(const struct commit *, unsigned long *size);

65 /*

66 * Get the commit's object contents, either from cache or by reading the object

67 * from disk. The resulting memory should not be modified, and must be given

68 * to unuse_commit_buffer when the caller is done.

69 */

70 -const void *get_commit_buffer(const struct commit *);

70 +const void *get_commit_buffer(const struct commit *, unsigned long *size);

72 /*

73 * Tell the commit subsytem that we are done with a particular commit buffer.

@@ -86,7 +86,7 @@ void free_commit_buffer(struct commit *);

86 * Disassociate any cached object buffer from the commit, but do not free it.

87 * The buffer (or NULL, if none) is returned.

88 */

89 -const void *detach_commit_buffer(struct commit *);

89 +const void *detach_commit_buffer(struct commit *, unsigned long *sizep);

91 /* Find beginning and length of commit subject. */

92 int find_commit_subject(const char *commit_buffer, const char **subject);

fsck.c

+1 -1

@@ -339,7 +339,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,

339

340 static int fsck_commit(struct commit *commit, fsck_error error_func)

341 {

342 - const char *buffer = get_commit_buffer(commit);

342 + const char *buffer = get_commit_buffer(commit, NULL);

343 int ret = fsck_commit_buffer(commit, buffer, error_func);

344 unuse_commit_buffer(commit, buffer);

345 return ret;

log-tree.c

+1 -1

@@ -588,7 +588,7 @@ void show_log(struct rev_info *opt)

588 show_mergetag(opt, commit);

589 }

590

591 - if (!get_cached_commit_buffer(commit))

591 + if (!get_cached_commit_buffer(commit, NULL))

592 return;

593

594 if (opt->show_notes) {

merge-recursive.c

+1 -1

@@ -190,7 +190,7 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)

190 printf(_("(bad commit)\n"));

191 else {

192 const char *title;

193 - const char *msg = get_commit_buffer(commit);

193 + const char *msg = get_commit_buffer(commit, NULL);

194 int len = find_commit_subject(msg, &title);

195 if (len)

196 printf("%.*s\n", len, title);

notes-merge.c

+1 -1

@@ -672,7 +672,7 @@ int notes_merge_commit(struct notes_merge_options *o,

672 DIR *dir;

673 struct dirent *e;

674 struct strbuf path = STRBUF_INIT;

675 - const char *buffer = get_commit_buffer(partial_commit);

675 + const char *buffer = get_commit_buffer(partial_commit, NULL);

676 const char *msg = strstr(buffer, "\n\n");

677 int baselen;

678

object.c

+2 -2

@@ -197,8 +197,8 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t

197 if (commit) {

198 if (parse_commit_buffer(commit, buffer, size))

199 return NULL;

200 - if (!get_cached_commit_buffer(commit)) {

201 - set_commit_buffer(commit, buffer);

200 + if (!get_cached_commit_buffer(commit, NULL)) {

201 + set_commit_buffer(commit, buffer, size);

202 *eaten_p = 1;

203 }

204 obj = &commit->object;

pretty.c

+2 -2

@@ -613,7 +613,7 @@ const char *logmsg_reencode(const struct commit *commit,

613 static const char *utf8 = "UTF-8";

614 const char *use_encoding;

615 char *encoding;

616 - const char *msg = get_commit_buffer(commit);

616 + const char *msg = get_commit_buffer(commit, NULL);

617 char *out;

618

619 if (!output_encoding || !*output_encoding) {

@@ -642,7 +642,7 @@ const char *logmsg_reencode(const struct commit *commit,

642 * the cached copy from get_commit_buffer, we need to duplicate it

643 * to avoid munging the cached copy.

644 */

645 - if (msg == get_cached_commit_buffer(commit))

645 + if (msg == get_cached_commit_buffer(commit, NULL))

646 out = xstrdup(msg);

647 else

648 out = (char *)msg;

sequencer.c

+1 -1

@@ -662,7 +662,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,

662 int subject_len;

663

664 for (cur = todo_list; cur; cur = cur->next) {

665 - const char *commit_buffer = get_commit_buffer(cur->item);

665 + const char *commit_buffer = get_commit_buffer(cur->item, NULL);

666 sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);

667 subject_len = find_commit_subject(commit_buffer, &subject);

668 strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,

sha1_name.c

+1 -1

@@ -869,7 +869,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,

869 commit = pop_most_recent_commit(&list, ONELINE_SEEN);

870 if (!parse_object(commit->object.sha1))

871 continue;

872 - buf = get_commit_buffer(commit);

872 + buf = get_commit_buffer(commit, NULL);

873 p = strstr(buf, "\n\n");

874 matches = p && !regexec(&regex, p + 2, 0, NULL, 0);

875 unuse_commit_buffer(commit, buf);