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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 13, 2022, 19:10 UTC
Message-ID: <221013.86fsfrleqi.gmgdl@evledraar.gmail.com>
In-Reply-To: <1fd58520253420fbe870a8528540dbc9e2178e3f.1665650564.git.gitgitgadget@gmail.com>

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

Show 5 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> 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.

Show 6 quoted lines

> @@ -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.

Show 10 quoted lines

>  		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...