Skip to content

Commit 351c6e7

Browse files
KarthikNayakgitster
authored andcommitted
refs/ref-cache: fix SEGFAULT when seeking in empty directories
The 'cache_ref_iterator_seek()' function is used to seek the `ref_iterator` to the desired reference in the ref-cache mechanism. We use the seeking functionality to implement the '--start-after' flag in 'git-for-each-ref(1)'. When using the files-backend with packed-refs, it is possible that some of the refs directories are empty. For e.g. just after repacking, the 'refs/heads' directory would be empty. The ref-cache seek mechanism, doesn't take this into consideration when descending into a subdirectory, and makes an out of bounds access, causing SEGFAULT as we try to access entries within the directory. Fix this by breaking out of the loop when we enter an empty directory. Since we start with the base directory of 'refs/' which is never empty, it is okay to perform this check after the first iteration in the `do..while` clause. Add tests which simulate this behavior and also provide coverage over using the feature over packed-refs. Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Karthik Nayak <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 821f583 commit 351c6e7

File tree

2 files changed

+66
-1
lines changed

2 files changed

+66
-1
lines changed

refs/ref-cache.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
539539
*/
540540
break;
541541
}
542-
} while (slash);
542+
} while (slash && dir->nr);
543543
}
544544

545545
return 0;

t/t6302-for-each-ref-filter.sh

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,4 +754,69 @@ test_expect_success 'start after used with custom sort order' '
754754
test_cmp expect actual
755755
'
756756

757+
test_expect_success 'start after with packed refs' '
758+
test_when_finished "rm -rf repo" &&
759+
git init repo &&
760+
(
761+
cd repo &&
762+
test_commit default &&
763+
764+
git update-ref --stdin <<-\EOF &&
765+
create refs/heads/branch @
766+
create refs/heads/side @
767+
create refs/odd/spot @
768+
create refs/tags/one @
769+
create refs/tags/two @
770+
commit
771+
EOF
772+
773+
cat >expect <<-\EOF &&
774+
refs/tags/default
775+
refs/tags/one
776+
refs/tags/two
777+
EOF
778+
779+
git pack-refs --all &&
780+
git for-each-ref --format="%(refname)" --start-after=refs/odd/spot >actual &&
781+
test_cmp expect actual
782+
)
783+
'
784+
785+
test_expect_success 'start after with packed refs and some loose refs' '
786+
test_when_finished "rm -rf repo" &&
787+
git init repo &&
788+
(
789+
cd repo &&
790+
test_commit default &&
791+
792+
git update-ref --stdin <<-\EOF &&
793+
create refs/heads/branch @
794+
create refs/heads/side @
795+
create refs/odd/spot @
796+
create refs/tags/one @
797+
create refs/tags/two @
798+
commit
799+
EOF
800+
801+
git pack-refs --all &&
802+
803+
git update-ref --stdin <<-\EOF &&
804+
create refs/heads/foo @
805+
create refs/odd/tee @
806+
commit
807+
EOF
808+
809+
cat >expect <<-\EOF &&
810+
refs/odd/tee
811+
refs/tags/default
812+
refs/tags/one
813+
refs/tags/two
814+
EOF
815+
816+
817+
git for-each-ref --format="%(refname)" --start-after=refs/odd/spot >actual &&
818+
test_cmp expect actual
819+
)
820+
'
821+
757822
test_done

0 commit comments

Comments
 (0)