Re: [PATCH v3 1/3] dir: change the scope of function 'directory_exists_in_index()'
From: Junio C Hamano <hidden>
Date: 2020-12-19 00:48:42
Johannes Schindelin [off-list ref] writes:
Hi Shourya, On Tue, 15 Dec 2020, Shourya Shukla wrote:quoted
Change the scope of the function 'directory_exists_in_index()' as well as declare it in 'dir.h'. Since the return type of the function is the enumerator 'exist_status', change its scope as well and declare it in 'dir.h'. While at it, rename the members of the aforementioned enum so as to avoid any naming clashes or confusions later on.This makes it sound as if only existing code was adjusted, in a minimal way, but no new code was introduced. But that's not true:
I noticed it last night, too---I suspect it was a mistake made while shuffling changes across steps with rebase -i.
quoted
Helped-by: Christian Couder [off-list ref] Helped-by: Kaartic Sivaraam [off-list ref] Signed-off-by: Shourya Shukla <redacted> --- builtin/submodule--helper.c | 408 ++++++++++++++++++++++++++++++++++++ dir.c | 30 ++- dir.h | 9 + 3 files changed, 429 insertions(+), 18 deletions(-)Tons of new code there. And unfortunately...
quoted
+static const char *parse_token(const char *cp, int *len) +{ + const char *p = cp, *start, *end; + char *str; + + start = p; + while (*p != ' ') + p++; + end = p; + str = xstrndup(start, end - start); + + while(*p == ' ') + p++; + + return str; +}This function is not careful enough to avoid buffer overruns. It even triggers a segmentation fault in our test suite: https://github.com/gitgitgadget/git/runs/1574891976?check_suite_focus=true#step:6:3152
I notice that len is not used at all ;-)
quoted hunk
I need this to make it pass (only tested locally so far, but I trust you to take the baton from here): -- snipsnap -- From c28c0cd3ac21d546394335957fbaa350ab287c3f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <redacted> Date: Sat, 19 Dec 2020 01:02:04 +0100 Subject: [PATCH] fixup??? dir: change the scope of function 'directory_exists_in_index()' This fixes the segmentation fault reported in the linux-musl job of our CI builds. Valgrind has this to say about it: ==32354== ==32354== Process terminating with default action of signal 11 (SIGSEGV) ==32354== Access not within mapped region at address 0x5C73000 ==32354== at 0x202F5A: parse_token (submodule--helper.c:2837) ==32354== by 0x20319B: report_fetch_remotes (submodule--helper.c:2871) ==32354== by 0x2033FD: add_submodule (submodule--helper.c:2898) ==32354== by 0x204612: module_add (submodule--helper.c:3146) ==32354== by 0x20478A: cmd_submodule__helper (submodule--helper.c:3202) ==32354== by 0x12655E: run_builtin (git.c:458) ==32354== by 0x1269B4: handle_builtin (git.c:712) ==32354== by 0x126C79: run_argv (git.c:779) ==32354== by 0x12715C: cmd_main (git.c:913) ==32354== by 0x2149A2: main (common-main.c:52) Signed-off-by: Johannes Schindelin <redacted> --- builtin/submodule--helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 4f1d892b9a9..29a6f80b937 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c@@ -2834,12 +2834,12 @@ static const char *parse_token(const char *cp, int *len) char *str; start = p; - while (*p != ' ') + while (*p && *p != ' ') p++; end = p; str = xstrndup(start, end - start); - while(*p == ' ') + while(*p && *p == ' ') p++; return str; --2.29.2.windows.1.1.g3464b98ce68