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 <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/submodule.c b/submodule.c
index 93e3fef..e72781d 100644 (file)
--- 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 d31b34d..51f8d0d 100755 (executable)
--- 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)' '