On Wed, Oct 16, 2024 at 08:21:23PM +0200, Wolfgang Müller wrote:
> Whilst git-shortlog(1) does not explicitly need any repository
> information when run without reference to one, it still parses some of
> its arguments with parse_revision_opt() which assumes that the hash
> algorithm is set. However, in c8aed5e8da (repository: stop setting SHA1
> as the default object hash, 2024-05-07) we stopped setting up a default
> hash algorithm and instead require commands to set it up explicitly.
>
> This was done for most other commands like in ab274909d4 (builtin/diff:
> explicitly set hash algo when there is no repo, 2024-05-07) but was
> missed for builtin/shortlog, making git-shortlog(1) segfault outside of
> a repository when given arguments like --author that trigger a call to
> parse_revision_opt().
Good analysis.
> Fix this for now by explicitly setting the hash algorithm to SHA1. Also
> add a regression test for the segfault.
Makes sense.
> Signed-off-by: Wolfgang Müller <wolf@oriole.systems>
> Thanks-to: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
In the future, please ensure that your Signed-off-by is the last trailer
in the section to indicate that you have certified everything above it
(which in that case would include your Thanks-to here).
> @@ -143,6 +143,10 @@ fuzz()
> test_grep "too many arguments" out
> '
>
> +test_expect_success 'shortlog --author from non-git directory does not segfault' '
> + echo | nongit git shortlog --author=author
> +'
> +
Instead of 'echo |', I wonder if it would be just as good to write this
test as:
nongit git shortlog --author=author </dev/null
Thanks,
Taylor
