Subject: Re: [PATCH v3 3/8] rebase: be stricter when reading state files containing oids

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 13 Oct 2022 21:10:53 +0200

On Thu, Oct 13 2022, Phillip Wood via GitGitGadget wrote:

> From: Phillip Wood <[email-masked]>
>
> The state files for 'onto' and 'orig_head' should contain a full hex
> oid, change the reading functions from get_oid() to get_oid_hex() to
> reflect this.

This seems sensible, but isn't "full hex oid" the tip of the iceberg in
get_oid() v.s. get_oid_hex() differences? The former allowing
e.g. :-syntax, ^-syntax etc.

> @@ -431,7 +431,7 @@ static int read_basic_state(struct rebase_options *opts)
>  	opts->head_name = starts_with(head_name.buf, "refs/") ?
>  		xstrdup(head_name.buf) : NULL;
>  	strbuf_release(&head_name);
> -	if (get_oid(buf.buf, &oid))
> +	if (get_oid_hex(buf.buf, &oid))

The change looks sensible, maybe a commit message update + assuring
ourselves that we don't care about anyone manually manipulating these
files (e.g. a script with OID abbreviation that would "echo" to these.

>  		return error(_("could not get 'onto': '%s'"), buf.buf);
>  	opts->onto = lookup_commit_or_die(&oid, buf.buf);
>  
> @@ -448,7 +448,7 @@ static int read_basic_state(struct rebase_options *opts)
>  	} else if (!read_oneliner(&buf, state_dir_path("head", opts),
>  				  READ_ONELINER_WARN_MISSING))
>  		return -1;
> -	if (get_oid(buf.buf, &opts->orig_head))
> +	if (get_oid_hex(buf.buf, &opts->orig_head))
>  		return error(_("invalid orig-head: '%s'"), buf.buf);

Not a new issue, but this error() is much more sensible than the above,
we could get "onto", we just didn't like its contents on error(), oh
well...