refs: change the internal reference-iteration API
Establish an internal API for iterating over references, which gives the callback functions direct access to the ref_entry structure describing the reference. (Do not change the iteration API that is exposed outside of the module.) Define a new internal callback signature int each_ref_entry_fn(struct ref_entry *entry, void *cb_data) Change do_for_each_ref_in_dir() and do_for_each_ref_in_dirs() to accept each_ref_entry_fn callbacks, and rename them to do_for_each_entry_in_dir() and do_for_each_entry_in_dirs(), respectively. Adapt their callers accordingly. Add a new function do_for_each_entry() analogous to do_for_each_ref() but using the new callback style. Change do_one_ref() into an each_ref_entry_fn that does some bookkeeping and then calls a wrapped each_ref_fn. Reimplement do_for_each_ref() in terms of do_for_each_entry(), using do_one_ref() as an adapter. Please note that the responsibility for setting current_ref remains in do_one_ref(), which means that current_ref is *not* set when iterating over references via the new internal API. This is not a disadvantage, because current_ref is not needed by callers of the internal API (they receive a pointer to the current ref_entry anyway). But more importantly, this change prevents peel_ref() from returning invalid results in the following scenario: When iterating via the external API, the iteration always includes both packed and loose references, and in particular never presents a packed ref if there is a loose ref with the same name. The internal API, on the other hand, gives the option to iterate over only the packed references. During such an iteration, there is no check whether the packed ref might be hidden by a loose ref of the same name. But until now the packed ref was recorded in current_ref during the iteration. So if peel_ref() were called with the reference name corresponding to current ref, it would return the peeled version of the packed ref even though there might be a loose ref that peels to a different value. This scenario doesn't currently occur in the code, but fix it to prevent things from breaking in a very confusing way in the future. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael Haggerty committed
Apr 22, 2013 at 21:52 UTC
624cac351478be90bf02a753f9de3e7dc80ca300
1 file changed
+83
-61
refs.c
+83
-61
@@ -557,22 +557,34 @@ static int ref_resolves_to_object(struct ref_entry *entry)
557
*/
558
static struct ref_entry *current_ref;
559
560
+typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);
561
+
562
+struct ref_entry_cb {
563
+ const char *base;
564
+ int trim;
565
+ int flags;
566
+ each_ref_fn *fn;
567
+ void *cb_data;
568
+};
569
+
570
/*
561
- * Handle one reference in a do_for_each_ref*()-style iteration.
571
+ * Handle one reference in a do_for_each_ref*()-style iteration,
572
+ * calling an each_ref_fn for each entry.
573
*/
563
-static int do_one_ref(const char *base, each_ref_fn fn, int trim,
564
- int flags, void *cb_data, struct ref_entry *entry)
574
+static int do_one_ref(struct ref_entry *entry, void *cb_data)
575
{
576
+ struct ref_entry_cb *data = cb_data;
577
int retval;
567
- if (prefixcmp(entry->name, base))
578
+ if (prefixcmp(entry->name, data->base))
579
return 0;
580
570
- if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
581
+ if (!(data->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
582
!ref_resolves_to_object(entry))
583
return 0;
584
585
current_ref = entry;
575
- retval = fn(entry->name + trim, entry->u.value.sha1, entry->flag, cb_data);
586
+ retval = data->fn(entry->name + data->trim, entry->u.value.sha1,
587
+ entry->flag, data->cb_data);
588
current_ref = NULL;
589
return retval;
590
}
@@ -581,11 +593,11 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim,
593
* Call fn for each reference in dir that has index in the range
594
* offset <= index < dir->nr. Recurse into subdirectories that are in
595
* that index range, sorting them before iterating. This function
584
- * does not sort dir itself; it should be sorted beforehand.
596
+ * does not sort dir itself; it should be sorted beforehand. fn is
597
+ * called for all references, including broken ones.
598
*/
586
-static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,
587
- const char *base,
588
- each_ref_fn fn, int trim, int flags, void *cb_data)
599
+static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
600
+ each_ref_entry_fn fn, void *cb_data)
601
{
602
int i;
603
assert(dir->sorted == dir->nr);
@@ -595,10 +607,9 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,
607
if (entry->flag & REF_DIR) {
608
struct ref_dir *subdir = get_ref_dir(entry);
609
sort_ref_dir(subdir);
598
- retval = do_for_each_ref_in_dir(subdir, 0,
599
- base, fn, trim, flags, cb_data);
610
+ retval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data);
611
} else {
601
- retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
612
+ retval = fn(entry, cb_data);
613
}
614
if (retval)
615
return retval;
@@ -611,12 +622,12 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset,
622
* by refname. Recurse into subdirectories. If a value entry appears
623
* in both dir1 and dir2, then only process the version that is in
624
* dir2. The input dirs must already be sorted, but subdirs will be
614
- * sorted as needed.
625
+ * sorted as needed. fn is called for all references, including
626
+ * broken ones.
627
*/
616
-static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
617
- struct ref_dir *dir2,
618
- const char *base, each_ref_fn fn, int trim,
619
- int flags, void *cb_data)
628
+static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
629
+ struct ref_dir *dir2,
630
+ each_ref_entry_fn fn, void *cb_data)
631
{
632
int retval;
633
int i1 = 0, i2 = 0;
@@ -627,12 +638,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
638
struct ref_entry *e1, *e2;
639
int cmp;
640
if (i1 == dir1->nr) {
630
- return do_for_each_ref_in_dir(dir2, i2,
631
- base, fn, trim, flags, cb_data);
641
+ return do_for_each_entry_in_dir(dir2, i2, fn, cb_data);
642
}
643
if (i2 == dir2->nr) {
634
- return do_for_each_ref_in_dir(dir1, i1,
635
- base, fn, trim, flags, cb_data);
644
+ return do_for_each_entry_in_dir(dir1, i1, fn, cb_data);
645
}
646
e1 = dir1->entries[i1];
647
e2 = dir2->entries[i2];
@@ -644,14 +653,13 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
653
struct ref_dir *subdir2 = get_ref_dir(e2);
654
sort_ref_dir(subdir1);
655
sort_ref_dir(subdir2);
647
- retval = do_for_each_ref_in_dirs(
648
- subdir1, subdir2,
649
- base, fn, trim, flags, cb_data);
656
+ retval = do_for_each_entry_in_dirs(
657
+ subdir1, subdir2, fn, cb_data);
658
i1++;
659
i2++;
660
} else if (!(e1->flag & REF_DIR) && !(e2->flag & REF_DIR)) {
661
/* Both are references; ignore the one from dir1. */
654
- retval = do_one_ref(base, fn, trim, flags, cb_data, e2);
662
+ retval = fn(e2, cb_data);
663
i1++;
664
i2++;
665
} else {
@@ -670,11 +678,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1,
678
if (e->flag & REF_DIR) {
679
struct ref_dir *subdir = get_ref_dir(e);
680
sort_ref_dir(subdir);
673
- retval = do_for_each_ref_in_dir(
674
- subdir, 0,
675
- base, fn, trim, flags, cb_data);
681
+ retval = do_for_each_entry_in_dir(
682
+ subdir, 0, fn, cb_data);
683
} else {
677
- retval = do_one_ref(base, fn, trim, flags, cb_data, e);
684
+ retval = fn(e, cb_data);
685
}
686
}
687
if (retval)
@@ -703,14 +710,13 @@ struct name_conflict_cb {
710
const char *conflicting_refname;
711
};
712
706
-static int name_conflict_fn(const char *existingrefname, const unsigned char *sha1,
707
- int flags, void *cb_data)
713
+static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
714
{
715
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
710
- if (data->oldrefname && !strcmp(data->oldrefname, existingrefname))
716
+ if (data->oldrefname && !strcmp(data->oldrefname, entry->name))
717
return 0;
712
- if (names_conflict(data->refname, existingrefname)) {
713
- data->conflicting_refname = existingrefname;
718
+ if (names_conflict(data->refname, entry->name)) {
719
+ data->conflicting_refname = entry->name;
720
return 1;
721
}
722
return 0;
@@ -718,7 +724,7 @@ static int name_conflict_fn(const char *existingrefname, const unsigned char *sh
724
725
/*
726
* Return true iff a reference named refname could be created without
721
- * conflicting with the name of an existing reference in array. If
727
+ * conflicting with the name of an existing reference in dir. If
728
* oldrefname is non-NULL, ignore potential conflicts with oldrefname
729
* (e.g., because oldrefname is scheduled for deletion in the same
730
* operation).
@@ -732,9 +738,7 @@ static int is_refname_available(const char *refname, const char *oldrefname,
738
data.conflicting_refname = NULL;
739
740
sort_ref_dir(dir);
735
- if (do_for_each_ref_in_dir(dir, 0, "", name_conflict_fn,
736
- 0, DO_FOR_EACH_INCLUDE_BROKEN,
737
- &data)) {
741
+ if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, &data)) {
742
error("'%s' exists; cannot create '%s'",
743
data.conflicting_refname, refname);
744
return 0;
@@ -1422,16 +1426,14 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
1426
}
1427
1428
/*
1425
- * Call fn for each reference in the specified submodule for which the
1426
- * refname begins with base. If trim is non-zero, then trim that many
1427
- * characters off the beginning of each refname before passing the
1428
- * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include
1429
- * broken references in the iteration. If fn ever returns a non-zero
1429
+ * Call fn for each reference in the specified submodule, omitting
1430
+ * references not in the containing_dir of base. fn is called for all
1431
+ * references, including broken ones. If fn ever returns a non-zero
1432
* value, stop the iteration and return that value; otherwise, return
1433
* 0.
1434
*/
1433
-static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
1434
- int trim, int flags, void *cb_data)
1435
+static int do_for_each_entry(const char *submodule, const char *base,
1436
+ each_ref_entry_fn fn, void *cb_data)
1437
{
1438
struct ref_cache *refs = get_ref_cache(submodule);
1439
struct ref_dir *packed_dir = get_packed_refs(refs);
@@ -1446,24 +1448,43 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
1448
if (packed_dir && loose_dir) {
1449
sort_ref_dir(packed_dir);
1450
sort_ref_dir(loose_dir);
1449
- retval = do_for_each_ref_in_dirs(
1450
- packed_dir, loose_dir,
1451
- base, fn, trim, flags, cb_data);
1451
+ retval = do_for_each_entry_in_dirs(
1452
+ packed_dir, loose_dir, fn, cb_data);
1453
} else if (packed_dir) {
1454
sort_ref_dir(packed_dir);
1454
- retval = do_for_each_ref_in_dir(
1455
- packed_dir, 0,
1456
- base, fn, trim, flags, cb_data);
1455
+ retval = do_for_each_entry_in_dir(
1456
+ packed_dir, 0, fn, cb_data);
1457
} else if (loose_dir) {
1458
sort_ref_dir(loose_dir);
1459
- retval = do_for_each_ref_in_dir(
1460
- loose_dir, 0,
1461
- base, fn, trim, flags, cb_data);
1459
+ retval = do_for_each_entry_in_dir(
1460
+ loose_dir, 0, fn, cb_data);
1461
}
1462
1463
return retval;
1464
}
1465
1466
+/*
1467
+ * Call fn for each reference in the specified submodule for which the
1468
+ * refname begins with base. If trim is non-zero, then trim that many
1469
+ * characters off the beginning of each refname before passing the
1470
+ * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include
1471
+ * broken references in the iteration. If fn ever returns a non-zero
1472
+ * value, stop the iteration and return that value; otherwise, return
1473
+ * 0.
1474
+ */
1475
+static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
1476
+ int trim, int flags, void *cb_data)
1477
+{
1478
+ struct ref_entry_cb data;
1479
+ data.base = base;
1480
+ data.trim = trim;
1481
+ data.flags = flags;
1482
+ data.fn = fn;
1483
+ data.cb_data = cb_data;
1484
+
1485
+ return do_for_each_entry(submodule, base, do_one_ref, &data);
1486
+}
1487
+
1488
static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
1489
{
1490
unsigned char sha1[20];
@@ -1873,20 +1894,21 @@ struct repack_without_ref_sb {
1894
int fd;
1895
};
1896
1876
-static int repack_without_ref_fn(const char *refname, const unsigned char *sha1,
1877
- int flags, void *cb_data)
1897
+static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data)
1898
{
1899
struct repack_without_ref_sb *data = cb_data;
1900
char line[PATH_MAX + 100];
1901
int len;
1902
1883
- if (!strcmp(data->refname, refname))
1903
+ if (!strcmp(data->refname, entry->name))
1904
return 0;
1905
+ if (!ref_resolves_to_object(entry))
1906
+ return 0; /* Skip broken refs */
1907
len = snprintf(line, sizeof(line), "%s %s\n",
1886
- sha1_to_hex(sha1), refname);
1908
+ sha1_to_hex(entry->u.value.sha1), entry->name);
1909
/* this should not happen but just being defensive */
1910
if (len > sizeof(line))
1889
- die("too long a refname '%s'", refname);
1911
+ die("too long a refname '%s'", entry->name);
1912
write_or_die(data->fd, line, len);
1913
return 0;
1914
}
@@ -1910,7 +1932,7 @@ static int repack_without_ref(const char *refname)
1932
}
1933
clear_packed_ref_cache(refs);
1934
packed = get_packed_refs(refs);
1913
- do_for_each_ref_in_dir(packed, 0, "", repack_without_ref_fn, 0, 0, &data);
1935
+ do_for_each_entry_in_dir(packed, 0, repack_without_ref_fn, &data);
1936
return commit_lock_file(&packlock);
1937
}
1938