builtin/sparse-checkout: fix leaking sanitized patterns

Both `git sparse-checkout add` and `git sparse-checkout set` accept a list of additional directories or patterns. These get massaged via calls to `sanitize_paths()`, which may end up modifying the passed-in array by updating its pointers to be prefixed paths. This allocates memory that we never free. Refactor the code to instead use a `struct strvec`, which makes it way easier for us to track the lifetime correctly. The couple of extra memory allocations likely do not matter as we only ever populate it with command line arguments. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Nov 20, 2024 at 14:39 UTC 58e7568c619cce872b17987f0d14b3de3703129a
1 file changed +39 -22
builtin/sparse-checkout.c
+39 -22
@@ -669,7 +669,7 @@ static void add_patterns_literal(int argc, const char **argv,
669 add_patterns_from_input(pl, argc, argv, use_stdin ? stdin : NULL);
670 }
671
672 -static int modify_pattern_list(int argc, const char **argv, int use_stdin,
672 +static int modify_pattern_list(struct strvec *args, int use_stdin,
673 enum modify_type m)
674 {
675 int result;
@@ -679,13 +679,13 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
679 switch (m) {
680 case ADD:
681 if (core_sparse_checkout_cone)
682 - add_patterns_cone_mode(argc, argv, pl, use_stdin);
682 + add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
683 else
684 - add_patterns_literal(argc, argv, pl, use_stdin);
684 + add_patterns_literal(args->nr, args->v, pl, use_stdin);
685 break;
686
687 case REPLACE:
688 - add_patterns_from_input(pl, argc, argv,
688 + add_patterns_from_input(pl, args->nr, args->v,
689 use_stdin ? stdin : NULL);
690 break;
691 }
@@ -706,12 +706,12 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
706 return result;
707 }
708
709 -static void sanitize_paths(int argc, const char **argv,
709 +static void sanitize_paths(struct strvec *args,
710 const char *prefix, int skip_checks)
711 {
712 int i;
713
714 - if (!argc)
714 + if (!args->nr)
715 return;
716
717 if (prefix && *prefix && core_sparse_checkout_cone) {
@@ -721,8 +721,11 @@ static void sanitize_paths(int argc, const char **argv,
721 */
722 int prefix_len = strlen(prefix);
723
724 - for (i = 0; i < argc; i++)
725 - argv[i] = prefix_path(prefix, prefix_len, argv[i]);
724 + for (i = 0; i < args->nr; i++) {
725 + char *prefixed_path = prefix_path(prefix, prefix_len, args->v[i]);
726 + strvec_replace(args, i, prefixed_path);
727 + free(prefixed_path);
728 + }
729 }
730
731 if (skip_checks)
@@ -732,20 +735,20 @@ static void sanitize_paths(int argc, const char **argv,
735 die(_("please run from the toplevel directory in non-cone mode"));
736
737 if (core_sparse_checkout_cone) {
735 - for (i = 0; i < argc; i++) {
736 - if (argv[i][0] == '/')
738 + for (i = 0; i < args->nr; i++) {
739 + if (args->v[i][0] == '/')
740 die(_("specify directories rather than patterns (no leading slash)"));
738 - if (argv[i][0] == '!')
741 + if (args->v[i][0] == '!')
742 die(_("specify directories rather than patterns. If your directory starts with a '!', pass --skip-checks"));
740 - if (strpbrk(argv[i], "*?[]"))
743 + if (strpbrk(args->v[i], "*?[]"))
744 die(_("specify directories rather than patterns. If your directory really has any of '*?[]\\' in it, pass --skip-checks"));
745 }
746 }
747
745 - for (i = 0; i < argc; i++) {
748 + for (i = 0; i < args->nr; i++) {
749 struct cache_entry *ce;
750 struct index_state *index = the_repository->index;
748 - int pos = index_name_pos(index, argv[i], strlen(argv[i]));
751 + int pos = index_name_pos(index, args->v[i], strlen(args->v[i]));
752
753 if (pos < 0)
754 continue;
@@ -754,9 +757,9 @@ static void sanitize_paths(int argc, const char **argv,
757 continue;
758
759 if (core_sparse_checkout_cone)
757 - die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), argv[i]);
760 + die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
761 else
759 - warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), argv[i]);
762 + warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
763 }
764 }
765
@@ -780,6 +783,8 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
783 N_("read patterns from standard in")),
784 OPT_END(),
785 };
786 + struct strvec patterns = STRVEC_INIT;
787 + int ret;
788
789 setup_work_tree();
790 if (!core_apply_sparse_checkout)
@@ -791,9 +796,14 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
796 builtin_sparse_checkout_add_options,
797 builtin_sparse_checkout_add_usage, 0);
798
794 - sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
799 + for (int i = 0; i < argc; i++)
800 + strvec_push(&patterns, argv[i]);
801 + sanitize_paths(&patterns, prefix, add_opts.skip_checks);
802
796 - return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
803 + ret = modify_pattern_list(&patterns, add_opts.use_stdin, ADD);
804 +
805 + strvec_clear(&patterns);
806 + return ret;
807 }
808
809 static char const * const builtin_sparse_checkout_set_usage[] = {
@@ -826,6 +836,8 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
836 PARSE_OPT_NONEG),
837 OPT_END(),
838 };
839 + struct strvec patterns = STRVEC_INIT;
840 + int ret;
841
842 setup_work_tree();
843 repo_read_index(the_repository);
@@ -846,13 +858,18 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
858 * top-level directory (much as 'init' would do).
859 */
860 if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
849 - argv = default_patterns;
850 - argc = default_patterns_nr;
861 + for (int i = 0; i < default_patterns_nr; i++)
862 + strvec_push(&patterns, default_patterns[i]);
863 } else {
852 - sanitize_paths(argc, argv, prefix, set_opts.skip_checks);
864 + for (int i = 0; i < argc; i++)
865 + strvec_push(&patterns, argv[i]);
866 + sanitize_paths(&patterns, prefix, set_opts.skip_checks);
867 }
868
855 - return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
869 + ret = modify_pattern_list(&patterns, set_opts.use_stdin, REPLACE);
870 +
871 + strvec_clear(&patterns);
872 + return ret;
873 }
874
875 static char const * const builtin_sparse_checkout_reapply_usage[] = {