From af6865a7f1e1d915d3b63e998226028ca4abb6ee Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Fri, 24 Mar 2017 17:36:06 -0700 Subject: [PATCH] submodule.c: convert is_submodule_modified to use strbuf_getwholeline Instead of implementing line reading yet again, make use of our beautiful library function to read one line. By using strbuf_getwholeline instead of strbuf_read, we avoid having to allocate memory for the entire child process output at once. That is, we limit maximum memory usage. Also we can start processing the output as it comes in, no need to wait for all of it. Once we know all information that we care about, we can terminate the child early. In that case we do not care about its exit code as well. By just closing our side of the pipe the child process will get a SIGPIPE signal, which it will not report nor do we report it in finish_command, ac78663b0d (run-command: don't warn on SIGPIPE deaths, 2015-12-29). Helped-by: Jonathan Nieder Signed-off-by: Stefan Beller Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- submodule.c | 30 ++++++++++++++---------------- t/t7506-status-submodule.sh | 18 +++++++++++++++++- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/submodule.c b/submodule.c index 93e3fefd3..e72781d9f 100644 --- a/submodule.c +++ b/submodule.c @@ -1041,12 +1041,12 @@ out: unsigned is_submodule_modified(const char *path, int ignore_untracked) { - ssize_t len; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; + FILE *fp; unsigned dirty_submodule = 0; - const char *line, *next_line; const char *git_dir; + int ignore_cp_exit_code = 0; strbuf_addf(&buf, "%s/.git", path); git_dir = read_gitfile(buf.buf); @@ -1072,29 +1072,27 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) if (start_command(&cp)) die("Could not run 'git status --porcelain' in submodule %s", path); - len = strbuf_read(&buf, cp.out, 1024); - line = buf.buf; - while (len > 2) { - if ((line[0] == '?') && (line[1] == '?')) + fp = xfdopen(cp.out, "r"); + while (strbuf_getwholeline(&buf, fp, '\n') != EOF) { + if ((buf.buf[0] == '?') && (buf.buf[1] == '?')) dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; else dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || - ignore_untracked)) - break; - - next_line = strchr(line, '\n'); - if (!next_line) + ignore_untracked)) { + /* + * We're not interested in any further information from + * the child any more, neither output nor its exit code. + */ + ignore_cp_exit_code = 1; break; - next_line++; - len -= (next_line - line); - line = next_line; + } } - close(cp.out); + fclose(fp); - if (finish_command(&cp)) + if (finish_command(&cp) && !ignore_cp_exit_code) die("'git status --porcelain' failed in submodule %s", path); strbuf_release(&buf); diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index d31b34da8..51f8d0d03 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -177,8 +177,24 @@ test_expect_success 'status with added file in modified submodule with .git file test_i18ngrep "modified: sub (new commits, modified content)" output ' +test_expect_success 'status with a lot of untracked files in the submodule' ' + ( + cd sub + i=0 && + while test $i -lt 1024 + do + >some-file-$i + i=$(( $i + 1 )) + done + ) && + git status --porcelain sub 2>err.actual && + test_must_be_empty err.actual && + rm err.actual +' + test_expect_success 'rm submodule contents' ' - rm -rf sub/* sub/.git + rm -rf sub && + mkdir sub ' test_expect_success 'status clean (empty submodule dir)' ' -- 2.11.0