@matheusbsbs / matheus-bagliano / commits / a59cfb3230

Fork 0

fsck: unify object-name code

Commit 90cf590f53 (fsck: optionally show more helpful info for broken links, 2016-07-17) added a system for decorating objects with names. The code is split across builtin/fsck.c (which gives the initial names) and fsck.c (which adds to the names as it traverses the object graph). This leads to some duplication, where both sites have near-identical describe_object() functions (the difference being that the one in builtin/fsck.c uses a circular array of buffers to allow multiple calls in a single printf). Let's provide a unified object_name API for fsck. That lets us drop the duplication, as well as making the interface boundaries more clear (which will let us refactor the implementation more in a future patch). We'll leave describe_object() in builtin/fsck.c as a thin wrapper around the new API, as it relies on a static global to make its many callers a bit shorter. We'll also convert the bare add_decoration() calls in builtin/fsck.c to put_object_name(). This fixes two minor bugs: 1. We leak many small strings. add_decoration() has a last-one-wins approach: it updates the decoration to the new string and returns the old one. But we ignore the return value, leaking the old string. This is quite common to trigger, since we look at reflogs: the tip of any ref will be described both by looking at the actual ref, as well as the latest reflog entry. So we'd always end up leaking one of those strings. 2. The last-one-wins approach gives us lousy names. For instance, we first look at all of the refs, and then all of the reflogs. So rather than seeing "refs/heads/master", we're likely to overwrite it with "HEAD@{12345678}". We're generally better off using the first name we find. And indeed, the test in t1450 expects this ugly HEAD@{} name. After this patch, we've switched to using fsck_put_object_name()'s first-one-wins semantics, and we output the more human-friendly "refs/tags/julius" (and the test is updated accordingly). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 18, 2019 at 00:56 UTC a59cfb32300baab00ee9cec68326309f4b2faca9

4 files changed +89 -69

builtin/fsck.c

+13 -37

@@ -52,24 +52,7 @@ static int name_objects;

53 static const char *describe_object(struct object *obj)

54 {

55 - static struct strbuf bufs[] = {

56 - STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT

57 - };

58 - static int b = 0;

59 - struct strbuf *buf;

60 - char *name = NULL;

61 -

62 - if (name_objects)

63 - name = lookup_decoration(fsck_walk_options.object_names, obj);

64 -

65 - buf = bufs + b;

66 - b = (b + 1) % ARRAY_SIZE(bufs);

67 - strbuf_reset(buf);

68 - strbuf_addstr(buf, oid_to_hex(&obj->oid));

69 - if (name)

70 - strbuf_addf(buf, " (%s)", name);

71 -

72 - return buf->buf;

55 + return fsck_describe_object(&fsck_walk_options, obj);

56 }

58 static const char *printable_type(struct object *obj)

@@ -499,10 +482,10 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,

482 if (!is_null_oid(oid)) {

483 obj = lookup_object(the_repository, oid);

484 if (obj && (obj->flags & HAS_OBJ)) {

502 - if (timestamp && name_objects)

503 - add_decoration(fsck_walk_options.object_names,

504 - obj,

505 - xstrfmt("%s@{%"PRItime"}", refname, timestamp));

485 + if (timestamp)

486 + fsck_put_object_name(&fsck_walk_options, obj,

487 + "%s@{%"PRItime"}",

488 + refname, timestamp);

489 obj->flags |= USED;

490 mark_object_reachable(obj);

491 } else if (!is_promisor_object(oid)) {

@@ -566,9 +549,8 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,

549 }

550 default_refs++;

551 obj->flags |= USED;

569 - if (name_objects)

570 - add_decoration(fsck_walk_options.object_names,

571 - obj, xstrdup(refname));

552 + fsck_put_object_name(&fsck_walk_options,

553 + obj, "%s", refname);

554 mark_object_reachable(obj);

555

556 return 0;

@@ -742,9 +724,7 @@ static int fsck_cache_tree(struct cache_tree *it)

724 return 1;

725 }

726 obj->flags |= USED;

745 - if (name_objects)

746 - add_decoration(fsck_walk_options.object_names,

747 - obj, xstrdup(":"));

727 + fsck_put_object_name(&fsck_walk_options, obj, ":");

728 mark_object_reachable(obj);

729 if (obj->type != OBJ_TREE)

730 err |= objerror(obj, _("non-tree in cache-tree"));

@@ -830,8 +810,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)

810 }

811

812 if (name_objects)

833 - fsck_walk_options.object_names =

834 - xcalloc(1, sizeof(struct decoration));

813 + fsck_enable_object_names(&fsck_walk_options);

814

815 git_config(fsck_config, NULL);

816

@@ -890,9 +869,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)

869 }

870

871 obj->flags |= USED;

893 - if (name_objects)

894 - add_decoration(fsck_walk_options.object_names,

895 - obj, xstrdup(arg));

872 + fsck_put_object_name(&fsck_walk_options, obj,

873 + "%s", arg);

874 mark_object_reachable(obj);

875 continue;

876 }

@@ -928,10 +906,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)

906 continue;

907 obj = &blob->object;

908 obj->flags |= USED;

931 - if (name_objects)

932 - add_decoration(fsck_walk_options.object_names,

933 - obj,

934 - xstrfmt(":%s", active_cache[i]->name));

909 + fsck_put_object_name(&fsck_walk_options, obj,

910 + ":%s", active_cache[i]->name);

911 mark_object_reachable(obj);

912 }

913 if (active_cache_tree)

fsck.c

+51 -31

@@ -312,15 +312,21 @@ static int report(struct fsck_options *options, struct object *object,

312 return result;

313 }

314

315 -static char *get_object_name(struct fsck_options *options, struct object *obj)

315 +void fsck_enable_object_names(struct fsck_options *options)

316 +{

317 + if (!options->object_names)

318 + options->object_names = xcalloc(1, sizeof(struct decoration));

319 +}

320 +

321 +const char *fsck_get_object_name(struct fsck_options *options, struct object *obj)

322 {

323 if (!options->object_names)

324 return NULL;

325 return lookup_decoration(options->object_names, obj);

326 }

327

322 -static void put_object_name(struct fsck_options *options, struct object *obj,

323 - const char *fmt, ...)

328 +void fsck_put_object_name(struct fsck_options *options, struct object *obj,

329 + const char *fmt, ...)

330 {

331 va_list ap;

332 struct strbuf buf = STRBUF_INIT;

@@ -337,17 +343,27 @@ static void put_object_name(struct fsck_options *options, struct object *obj,

343 va_end(ap);

344 }

345

340 -static const char *describe_object(struct fsck_options *o, struct object *obj)

346 +const char *fsck_describe_object(struct fsck_options *options,

347 + struct object *obj)

348 {

342 - static struct strbuf buf = STRBUF_INIT;

343 - char *name;

344 -

345 - strbuf_reset(&buf);

346 - strbuf_addstr(&buf, oid_to_hex(&obj->oid));

347 - if (o->object_names && (name = lookup_decoration(o->object_names, obj)))

348 - strbuf_addf(&buf, " (%s)", name);

349 + static struct strbuf bufs[] = {

350 + STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT

351 + };

352 + static int b = 0;

353 + struct strbuf *buf;

354 + char *name = NULL;

355 +

356 + if (options->object_names)

357 + name = lookup_decoration(options->object_names, obj);

358 +

359 + buf = bufs + b;

360 + b = (b + 1) % ARRAY_SIZE(bufs);

361 + strbuf_reset(buf);

362 + strbuf_addstr(buf, oid_to_hex(&obj->oid));

363 + if (name)

364 + strbuf_addf(buf, " (%s)", name);

365

350 - return buf.buf;

366 + return buf->buf;

367 }

368

369 static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *options)

@@ -360,7 +376,7 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op

376 if (parse_tree(tree))

377 return -1;

378

363 - name = get_object_name(options, &tree->object);

379 + name = fsck_get_object_name(options, &tree->object);

380 if (init_tree_desc_gently(&desc, tree->buffer, tree->size))

381 return -1;

382 while (tree_entry_gently(&desc, &entry)) {

@@ -373,20 +389,21 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op

389 if (S_ISDIR(entry.mode)) {

390 obj = (struct object *)lookup_tree(the_repository, &entry.oid);

391 if (name && obj)

376 - put_object_name(options, obj, "%s%s/", name,

377 - entry.path);

392 + fsck_put_object_name(options, obj, "%s%s/",

393 + name, entry.path);

394 result = options->walk(obj, OBJ_TREE, data, options);

395 }

396 else if (S_ISREG(entry.mode) || S_ISLNK(entry.mode)) {

397 obj = (struct object *)lookup_blob(the_repository, &entry.oid);

398 if (name && obj)

383 - put_object_name(options, obj, "%s%s", name,

384 - entry.path);

399 + fsck_put_object_name(options, obj, "%s%s",

400 + name, entry.path);

401 result = options->walk(obj, OBJ_BLOB, data, options);

402 }

403 else {

404 result = error("in tree %s: entry %s has bad mode %.6o",

389 - describe_object(options, &tree->object), entry.path, entry.mode);

405 + fsck_describe_object(options, &tree->object),

406 + entry.path, entry.mode);

407 }

408 if (result < 0)

409 return result;

@@ -407,10 +424,10 @@ static int fsck_walk_commit(struct commit *commit, void *data, struct fsck_optio

424 if (parse_commit(commit))

425 return -1;

426

410 - name = get_object_name(options, &commit->object);

427 + name = fsck_get_object_name(options, &commit->object);

428 if (name)

412 - put_object_name(options, &get_commit_tree(commit)->object,

413 - "%s:", name);

429 + fsck_put_object_name(options, &get_commit_tree(commit)->object,

430 + "%s:", name);

431

432 result = options->walk((struct object *)get_commit_tree(commit),

433 OBJ_TREE, data, options);

@@ -441,13 +458,15 @@ static int fsck_walk_commit(struct commit *commit, void *data, struct fsck_optio

458 struct object *obj = &parents->item->object;

459

460 if (counter++)

444 - put_object_name(options, obj, "%s^%d",

445 - name, counter);

461 + fsck_put_object_name(options, obj, "%s^%d",

462 + name, counter);

463 else if (generation > 0)

447 - put_object_name(options, obj, "%.*s~%d",

448 - name_prefix_len, name, generation + 1);

464 + fsck_put_object_name(options, obj, "%.*s~%d",

465 + name_prefix_len, name,

466 + generation + 1);

467 else

450 - put_object_name(options, obj, "%s^", name);

468 + fsck_put_object_name(options, obj, "%s^",

469 + name);

470 }

471 result = options->walk((struct object *)parents->item, OBJ_COMMIT, data, options);

472 if (result < 0)

@@ -461,12 +480,12 @@ static int fsck_walk_commit(struct commit *commit, void *data, struct fsck_optio

480

481 static int fsck_walk_tag(struct tag *tag, void *data, struct fsck_options *options)

482 {

464 - char *name = get_object_name(options, &tag->object);

483 + const char *name = fsck_get_object_name(options, &tag->object);

484

485 if (parse_tag(tag))

486 return -1;

487 if (name)

469 - put_object_name(options, tag->tagged, "%s", name);

488 + fsck_put_object_name(options, tag->tagged, "%s", name);

489 return options->walk(tag->tagged, OBJ_ANY, data, options);

490 }

491

@@ -488,7 +507,8 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options)

507 case OBJ_TAG:

508 return fsck_walk_tag((struct tag *)obj, data, options);

509 default:

491 - error("Unknown object type for %s", describe_object(options, obj));

510 + error("Unknown object type for %s",

511 + fsck_describe_object(options, obj));

512 return -1;

513 }

514 }

@@ -962,10 +982,10 @@ int fsck_error_function(struct fsck_options *o,

982 struct object *obj, int msg_type, const char *message)

983 {

984 if (msg_type == FSCK_WARN) {

965 - warning("object %s: %s", describe_object(o, obj), message);

985 + warning("object %s: %s", fsck_describe_object(o, obj), message);

986 return 0;

987 }

968 - error("object %s: %s", describe_object(o, obj), message);

988 + error("object %s: %s", fsck_describe_object(o, obj), message);

989 return 1;

990 }

991

fsck.h

+24

@@ -67,4 +67,28 @@ int fsck_object(struct object *obj, void *data, unsigned long size,

67 */

68 int fsck_finish(struct fsck_options *options);

70 +/*

71 + * Subsystem for storing human-readable names for each object.

72 + *

73 + * If fsck_enable_object_names() has not been called, all other functions are

74 + * noops.

75 + *

76 + * Use fsck_put_object_name() to seed initial names (e.g. from refnames); the

77 + * fsck code will extend that while walking trees, etc.

78 + *

79 + * Use fsck_get_object_name() to get a single name (or NULL if none). Or the

80 + * more convenient describe_object(), which always produces an output string

81 + * with the oid combined with the name (if any). Note that the return value

82 + * points to a rotating array of static buffers, and may be invalidated by a

83 + * subsequent call.

84 + */

85 +void fsck_enable_object_names(struct fsck_options *options);

86 +const char *fsck_get_object_name(struct fsck_options *options,

87 + struct object *obj);

88 +__attribute__((format (printf,3,4)))

89 +void fsck_put_object_name(struct fsck_options *options, struct object *obj,

90 + const char *fmt, ...);

91 +const char *fsck_describe_object(struct fsck_options *options,

92 + struct object *obj);

93 +

94 #endif

t/t1450-fsck.sh

+1 -1

@@ -616,7 +616,7 @@ test_expect_success 'fsck --name-objects' '

616 remove_object $(git rev-parse julius:caesar.t) &&

617 test_must_fail git fsck --name-objects >out &&

618 tree=$(git rev-parse --verify julius:) &&

619 - test_i18ngrep -E "$tree \((refs/heads/master|HEAD)@\{[0-9]*\}:" out

619 + test_i18ngrep "$tree (refs/tags/julius:" out

620 )

621 '

622