[PATCH v2 0/3] config: fix segfault when parsing "core.abbrev" without repo

Hi,

this is the second version of my patch series that fixes a segfault when parsing "core.abbrev" without a repository.

Changes compared to v1:

  - Stop truncating the abbreviation length to the upper boundary
    completely. It's unnecessary, complicates the code and makes us
    dependent on `the_repository`.

  - Adapt `parse_opt_abbrev_cb()` to also stop truncating such that the
    behaviour of "core.abbrev" and `--abbrev` is the same.

  - Extend test coverage a bit.

Thanks!

Patrick

Patrick Steinhardt (3):
  config: fix segfault when parsing "core.abbrev" without repo
  parse-options-cb: stop clamping "--abbrev=" to hash length
  object-name: don't try to abbreviate to lengths greater than hexsz

 config.c           |  4 ++--
 object-name.c      |  2 +-
 parse-options-cb.c |  2 --
 t/t4202-log.sh     | 24 ++++++++++++++++++++++++
 t/t5601-clone.sh   |  7 +++++++
 5 files changed, 34 insertions(+), 5 deletions(-)

Range-diff against v1:
1:  7ded51bbce ! 1:  b48c50dd92 config: fix segfault when parsing "core.abbrev" without repo
    @@ Commit message
         `the_hash_algo` outside of Git repositories.
     
         Fix both of these issues by not making it an error anymore when the
    -    given length exceeds the hash length. Instead, if we have a repository,
    -    then we truncate the length to the maximum length of `the_hash_algo`.
    -    Otherwise, we simply leave the abbreviated length intact and store it
    -    as-is. This is equivalent to the logic in `parse_opt_abbrev_cb()` and is
    -    handled just fine by `repo_find_unique_abbrev_r()`. In practice, we
    -    should never even end up using `default_abbrev` without a repository
    -    anyway given that abbreviating object IDs to unique prefixes requires us
    -    to have access to an object database.
    +    given length exceeds the hash length. Instead, leave the abbreviated
    +    length intact. `repo_find_unique_abbrev_r()` handles this just fine
    +    except for a performance penalty which we will fix in a subsequent
    +    commit.
     
         Reported-by: Kyle Lippincott <spectral@google.com>
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
    @@ config.c: static int git_default_core_config(const char *var, const char *value,
      			default_abbrev = -1;
      		else if (!git_parse_maybe_bool_text(value))
     -			default_abbrev = the_hash_algo->hexsz;
    -+			default_abbrev = startup_info->have_repository ?
    -+				the_hash_algo->hexsz : GIT_MAX_HEXSZ;
    ++			default_abbrev = GIT_MAX_HEXSZ;
      		else {
      			int abbrev = git_config_int(var, value, ctx->kvi);
     -			if (abbrev < minimum_abbrev || abbrev > the_hash_algo->hexsz)
     +			if (abbrev < minimum_abbrev)
      				return error(_("abbrev length out of range: %d"), abbrev);
    -+			else if (startup_info->have_repository && abbrev > the_hash_algo->hexsz)
    -+				abbrev = the_hash_algo->hexsz;
      			default_abbrev = abbrev;
      		}
    - 		return 0;
     
      ## t/t4202-log.sh ##
     @@ t/t4202-log.sh: test_expect_success 'log.abbrevCommit configuration' '
      	test_cmp expect.whatchanged.full actual
      '
      
    -+test_expect_success 'log.abbrevCommit with --abbrev=9000' '
    ++test_expect_success '--abbrev-commit with core.abbrev=false' '
     +	git log --no-abbrev >expect &&
    -+	git log --abbrev-commit --abbrev=9000 >actual &&
    ++	git -c core.abbrev=false log --abbrev-commit >actual &&
    ++	test_cmp expect actual
    ++'
    ++
    ++test_expect_success '--abbrev-commit with core.abbrev=9000' '
    ++	git log --no-abbrev >expect &&
    ++	git -c core.abbrev=9000 log --abbrev-commit >actual &&
     +	test_cmp expect actual
     +'
     +
-:  ---------- > 2:  92860256a6 parse-options-cb: stop clamping "--abbrev=" to hash length
2:  31c0405f85 = 3:  0ccb8d8efa object-name: don't try to abbreviate to lengths greater than hexsz