Re: [PATCH v2 05/24] refs: update ref transactions to use struct object_id
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Oct 9, 2017, 23:13 UTC
- Message-ID
- <20171009231304.GI19555@aiede.mtv.corp.google.com>
- In-Reply-To
- <20171009011132.675341-6-sandals@crustytoothpaste.net>
brian m. carlson wrote:
Show 23 quoted lines
> Update the ref transaction code to use struct object_id. Remove one > NULL pointer check which was previously inserted around a dereference; > since we now pass a pointer to struct object_id directly through, the > code we're calling handles this for us. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > branch.c | 2 +- > builtin/clone.c | 2 +- > builtin/commit.c | 4 ++-- > builtin/fetch.c | 4 ++-- > builtin/receive-pack.c | 4 ++-- > builtin/replace.c | 2 +- > builtin/tag.c | 2 +- > builtin/update-ref.c | 8 ++++---- > fast-import.c | 4 ++-- > refs.c | 44 +++++++++++++++++++++----------------------- > refs.h | 24 ++++++++++++------------ > refs/files-backend.c | 12 ++++++------ > refs/refs-internal.h | 4 ++-- > sequencer.c | 2 +- > walker.c | 2 +- > 15 files changed, 59 insertions(+), 61 deletions(-)
Makes sense.
[...]
> +++ b/refs.c
[...]
Show 10 quoted lines
> int ref_transaction_create(struct ref_transaction *transaction,
> const char *refname,
> - const unsigned char *new_sha1,
> + const struct object_id *new_oid,
> unsigned int flags, const char *msg,
> struct strbuf *err)
> {
> - if (!new_sha1 || is_null_sha1(new_sha1))
> + if (!new_oid || is_null_oid(new_oid))
> die("BUG: create called without valid new_sha1");The error message still refers to new_sha1.
[...]
Show 10 quoted lines
> int ref_transaction_delete(struct ref_transaction *transaction,
> const char *refname,
> - const unsigned char *old_sha1,
> + const struct object_id *old_oid,
> unsigned int flags, const char *msg,
> struct strbuf *err)
> {
> - if (old_sha1 && is_null_sha1(old_sha1))
> + if (old_oid && is_null_oid(old_oid))
> die("BUG: delete called with old_sha1 set to zeros");Likewise.
[...]
Show 10 quoted lines
> int ref_transaction_verify(struct ref_transaction *transaction,
> const char *refname,
> - const unsigned char *old_sha1,
> + const struct object_id *old_oid,
> unsigned int flags,
> struct strbuf *err)
> {
> - if (!old_sha1)
> + if (!old_oid)
> die("BUG: verify called with old_sha1 set to NULL");Likewise.
[...]
> +++ b/refs.h > @@ -519,15 +519,15 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
[...]
Show 11 quoted lines
> /* > - * Add a reference creation to transaction. new_sha1 is the value that > + * Add a reference creation to transaction. new_oid is the value that > * the reference should have after the update; it must not be > - * null_sha1. It is verified that the reference does not exist > + * null_oid. It is verified that the reference does not exist > * already. > * > * See the above comment "Reference transaction updates" for more > @@ -535,35 +535,35 @@ int ref_transaction_update(struct ref_transaction *transaction, > */
(Possible diff generation bug: there's exactly one line represented in that @@ field. I would have expected the diff generator to combine the hunks.)
I think this is fine to go in without the error messages being fixed. A grep for 'sha1' will find them later so that they can be addressed in a separate patch.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>