Patrick Steinhardt <ps@xxxxxx> writes: > The `validate_headref()` function takes a path to a supposed "HEAD" file > and checks whether its format is something that we understand. It is > used as part of our repository discovery to check whether a specific > directory is a Git directory or not. > > Part of the validation is a check for a detached HEAD that contains a > plain object ID. To do this validation we use `get_oid_hex()`, which > relies on `the_hash_algo`. At this point in time the hash algo cannot > yet be initialized though because we didn't yet read the Git config. > Consequently, it will always be the SHA1 hash algorithm. > > In practice this works alright because `get_oid_hex()` only ends up > checking whether the prefix of the buffer is a valid object ID. And > because SHA1 is shorter than SHA256, the function will successfully > parse SHA256 object IDs, as well. > > It is somewhat fragile though and not really the intent to only check > for SHA1. With this in mind, harden the code to use `get_oid_hex_any()` > to check whether the "HEAD" file parses as any known hash. All makes sense, and given the above, I strongly suspect that we would want to make the validate_headref() function a file-scope static in setup.c to make sure it is only called/callable from the repository discovery codepath. Especially that if somebody calls this function again after we find out that the repository uses SHA-256, we would want to let the caller know that the detached HEAD records SHA-1 and we are in an inconsistent state. > It follows that we could just leave the current code intact, as in > practice the code change doesn't have any user visible impact. But it > also prepares us for `the_hash_algo` being unset when there is no > repositroy. Or perhaps we use get_oid_hex_any() != GIT_HASH_UNKNOWN when the_hash_algo has not been determined, and use !get_oid_hex() after we have determined which algorithm we are using? It may be what you did in a later step in the series, so let me read on. Thanks. > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > path.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/path.c b/path.c > index 67229edb9c..cc02165530 100644 > --- a/path.c > +++ b/path.c > @@ -693,7 +693,7 @@ int validate_headref(const char *path) > /* > * Is this a detached HEAD? > */ > - if (!get_oid_hex(buffer, &oid)) > + if (get_oid_hex_any(buffer, &oid) != GIT_HASH_UNKNOWN) > return 0; > > return -1;
