Fix null dereferences on unset format strings

[krobelus]

Our argv_format() reports success but leaves the output at null
when the input is "%(cmdlineargs)" and the corresponding option
("opt_cmdline_args") is null.
Other callers already check if the output is null, do the same.

Fixes two null dereferences in prompt.c:

:!%(cmdlineargs)
:echo %(cmdlineargs)

I don't know if there is a reproducer for the change in argv.c but
this seems better.

[koutcher]

Why not doing it in one line with:

diff --git a/src/argv.c b/src/argv.c
index de0bcf4..27e6d31 100644
--- a/src/argv.c
+++ b/src/argv.c
@@ -472,7 +472,7 @@ argv_format(struct argv_env *argv_env, const char ***dst_argv, const char *src_a
                }
        }

-       return src_argv[argc] == NULL;
+       return src_argv[argc] == NULL && *dst_argv;
 }

[krobelus]

Ok the second version does that, which makes things simpler.
It makes :echo %(cmdlineargs) fail, whereas the first version just made that print the empty string.
That probably doesn't matter since it's such an academic case.

[koutcher]

I think we shouldn't fail on an empty arg. That should do the trick:

diff --git a/src/argv.c b/src/argv.c
index 27e6d31..2fc0d0a 100644
--- a/src/argv.c
+++ b/src/argv.c
@@ -361,7 +361,7 @@ format_append_argv(struct format_context *format, const char ***dst_argv, const
        int argc;

        if (!src_argv)
-               return true;
+               return argv_append(dst_argv, "");

        for (argc = 0; src_argv[argc]; argc++)
                if (!format_append_arg(format, dst_argv, src_argv[argc]))

[krobelus]

Wow how could I miss that! Updated.

[jonas]

I suggest adding the following test case (sorry not the best way to provide this diff).

> make test/regressions/github-1136-test
      TEST  test/regressions/github-1136-test
      CASE  test/regressions/github-1136-test:bang-cmdlineargs-doesnt-crash
/Users/jonas.fonseca/src/tig/test/tools/libtest.sh: line 707: 39404 Segmentation fault: 11  "$runner" tig "$@" 0<&4 > "$HOME/${prefix}stdout" 2> "$HOME/${prefix}stderr.orig"
      CASE  test/regressions/github-1136-test:echo-cmdlineargs-doesnt-crash
/Users/jonas.fonseca/src/tig/test/tools/libtest.sh: line 707: 39466 Segmentation fault: 11  "$runner" tig "$@" 0<&4 > "$HOME/${prefix}stdout" 2> "$HOME/${prefix}stderr.orig"
            | Failed 4 out of 6 test(s)
            | [FAIL] unexpected status code: 139 (should be 0)
            | [FAIL] bang-cmdlineargs-doesnt-crash.screen not found
            |   [OK] bang-cmdlineargs-doesnt-crash.stderr assertion
            | [FAIL] unexpected status code: 139 (should be 0)
            | [FAIL] echo-cmdlineargs-doesnt-crash.screen not found
            |   [OK] echo-cmdlineargs-doesnt-crash.stderr assertion

diff --git a/test/regressions/github-1136-test b/test/regressions/github-1136-test
new file mode 100755
index 0000000..25f1271
--- /dev/null
+++ b/test/regressions/github-1136-test
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+. libtest.sh
+. libgit.sh
+
+LINES=10
+
+in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"
+
+test_case bang-cmdlineargs-doesnt-crash \
+	--args='status' \
+	--script='
+	:!%(cmdlineargs)
+	' <<EOF
+On branch master
+Changes to be committed:
+  (no files)
+Changes not staged for commit:
+  (no files)
+Untracked files:
+  (no files)
+
+[status] Nothing to update                                                  100%
+EOF
+
+test_case echo-cmdlineargs-doesnt-crash \
+	--args='status' \
+	--script='
+	:echo %(cmdlineargs)
+	' <<EOF
+On branch master
+Changes to be committed:
+  (no files)
+Changes not staged for commit:
+  (no files)
+Untracked files:
+  (no files)
+
+[status] Nothing to update                                                  100%
+EOF
+
+run_test_cases

[jonas]

If you prefer I can do it after merging. Just wanted to see if this could be tested.

[jonas]

Thanks