On Wed, Oct 11, 2017 at 08:33:46AM +0200, Michael Haggerty wrote:
> On 10/09/2017 03:11 AM, brian m. carlson wrote:
> > diff --git a/refs.c b/refs.c
> > index 0a5b68d6fb..51942df7b3 100644
> > --- a/refs.c
> > +++ b/refs.c
> > [...]
> > @@ -1003,12 +995,12 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
> > int ret = 0;
> >
> > if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
> > - assert(refs == get_main_ref_store());
>
> Was the deletion of the line above intentional?
No, that would not have been intentional. (I would have mentioned it in
the commit message if it were.) I probably accidentally deleted a line
in my editor. Will fix.
> > - ret = write_pseudoref(refname, new_sha1, old_sha1, &err);
> > + ret = write_pseudoref(refname, new_oid, old_oid, &err);
>
> This is not new to your code, but I just noticed a problem here.
> `refs_update_ref()` is documented, via its reference to
> `ref_transaction_update()`, to allow `new_sha1` (i.e., now `new_oid`) to
> be NULL. (NULL signifies that the value of the reference shouldn't be
> changed.)
>
> But `write_pseudoref()` dereferences its `oid` argument unconditionally,
> so this call would fail if `new_oid` is NULL.
>
> This has all been the case since `write_pseudoref()` was introduced in
> 74ec19d4be (pseudorefs: create and use pseudoref update and delete
> functions, 2015-07-31).
>
> In my opinion, `write_pseudoref()` is broken. It should accept NULL as
> its `oid` argument.
I can stuff a patch in for that.
--
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
