From b0ca411051d8444b4b32bda3fdd0aba871035d2a Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:01 +0100 Subject: files_transaction_prepare(): don't leak flags to packed transaction The files backend uses `ref_update::flags` for several internal flags. But those flags have no meaning to the packed backend. So when adding updates for the packed-refs transaction, only use flags that make sense to the packed backend. `REF_NODEREF` is part of the public interface, and it's logically what we want, so include it. In fact it is actually ignored by the packed backend (which doesn't support symbolic references), but that's its own business. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs/files-backend.c b/refs/files-backend.c index 2bd54e1..fadf103 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2594,8 +2594,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, ref_transaction_add_update( packed_transaction, update->refname, - update->flags & ~REF_HAVE_OLD, - &update->new_oid, &update->old_oid, + REF_HAVE_NEW | REF_NODEREF, + &update->new_oid, NULL, NULL); } } -- cgit v0.10.2-6-g49f6 From b00f3cfa92d10d7180e6baf01d570eb904b5a592 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:02 +0100 Subject: prune_ref(): call `ref_transaction_add_update()` directly `prune_ref()` needs to use the `REF_ISPRUNING` flag, but we want to make that flag private to the files backend. So instead of calling `ref_transaction_delete()`, which is a public function and therefore shouldn't allow the `REF_ISPRUNING` flag, change `prune_ref()` to call `ref_transaction_add_update()`, which is private to the refs module. (Note that we don't need any of the other services provided by `ref_transaction_delete()`.) This allows us to change `ref_transaction_update()` to reject the `REF_ISPRUNING` flag. Do so by adjusting `REF_TRANSACTION_UPDATE_ALLOWED_FLAGS`. Also add parentheses to its definition to avoid potential future mishaps. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs.h b/refs.h index 15fd419..4ffef95 100644 --- a/refs.h +++ b/refs.h @@ -349,9 +349,7 @@ int refs_pack_refs(struct ref_store *refs, unsigned int flags); * Flags that can be passed in to ref_transaction_update */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - REF_ISPRUNING | \ - REF_FORCE_CREATE_REFLOG | \ - REF_NODEREF + (REF_NODEREF | REF_FORCE_CREATE_REFLOG) /* * Setup reflog before using. Fill in err and return -1 on failure. diff --git a/refs/files-backend.c b/refs/files-backend.c index fadf103..ba72d28 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -989,22 +989,29 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; + int ret = -1; if (check_refname_format(r->name, 0)) return; transaction = ref_store_transaction_begin(&refs->base, &err); - if (!transaction || - ref_transaction_delete(transaction, r->name, &r->oid, - REF_ISPRUNING | REF_NODEREF, NULL, &err) || - ref_transaction_commit(transaction, &err)) { - ref_transaction_free(transaction); + if (!transaction) + goto cleanup; + ref_transaction_add_update( + transaction, r->name, + REF_NODEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_ISPRUNING, + &null_oid, &r->oid, NULL); + if (ref_transaction_commit(transaction, &err)) + goto cleanup; + + ret = 0; + +cleanup: + if (ret) error("%s", err.buf); - strbuf_release(&err); - return; - } - ref_transaction_free(transaction); strbuf_release(&err); + ref_transaction_free(transaction); + return; } /* -- cgit v0.10.2-6-g49f6 From a9bbbcec0d863d719dd4ae39fc2242b32c2008e7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:03 +0100 Subject: ref_transaction_update(): die on disallowed flags Callers shouldn't be passing disallowed flags into `ref_transaction_update()`. So instead of masking them off, treat it as a bug if any are set. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 62a7621..7c1e206 100644 --- a/refs.c +++ b/refs.c @@ -940,7 +940,8 @@ int ref_transaction_update(struct ref_transaction *transaction, return -1; } - flags &= REF_TRANSACTION_UPDATE_ALLOWED_FLAGS; + if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS) + BUG("illegal flags 0x%x passed to ref_transaction_update()", flags); flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0); -- cgit v0.10.2-6-g49f6 From 62c72d1fd0aa39429011b76ff5b1953a561e6581 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:04 +0100 Subject: ref_transaction_add_update(): remove a check We want to make `REF_ISPRUNING` internal to the files backend. For this to be possible, `ref_transaction_add_update()` mustn't know about it. So move the check that `REF_ISPRUNING` is only used with `REF_NODEREF` from this function to `files_transaction_prepare()`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 7c1e206..0d9a134 100644 --- a/refs.c +++ b/refs.c @@ -906,9 +906,6 @@ struct ref_update *ref_transaction_add_update( if (transaction->state != REF_TRANSACTION_OPEN) die("BUG: update called for transaction that is not open"); - if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF)) - die("BUG: REF_ISPRUNING set without REF_NODEREF"); - FLEX_ALLOC_STR(update, refname, refname); ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc); transaction->updates[transaction->nr++] = update; diff --git a/refs/files-backend.c b/refs/files-backend.c index ba72d28..a47771e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2518,13 +2518,18 @@ static int files_transaction_prepare(struct ref_store *ref_store, * transaction. (If we end up splitting up any updates using * split_symref_update() or split_head_update(), those * functions will check that the new updates don't have the - * same refname as any existing ones.) + * same refname as any existing ones.) Also fail if any of the + * updates use REF_ISPRUNING without REF_NODEREF. */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; struct string_list_item *item = string_list_append(&affected_refnames, update->refname); + if ((update->flags & REF_ISPRUNING) && + !(update->flags & REF_NODEREF)) + BUG("REF_ISPRUNING set without REF_NODEREF"); + /* * We store a pointer to update in item->util, but at * the moment we never use the value of this field -- cgit v0.10.2-6-g49f6 From 5ac95fee3d6f77867a627a713f9aa72dc32be18f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:05 +0100 Subject: refs: tidy up and adjust visibility of the `ref_update` flags The constants used for `ref_update::flags` were rather disorganized: * The definitions in `refs.h` were not close to the functions that used them. * Maybe constants were defined in `refs-internal.h`, making them visible to the whole refs module, when in fact they only made sense for the files backend. * Their documentation wasn't very consistent and partly still referred to sha1s rather than oids. * The numerical values followed no rational scheme Fix all of these problems. The main functional improvement is that some constants' visibility is now limited to `files-backend.c`. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs.h b/refs.h index 4ffef95..261d46c 100644 --- a/refs.h +++ b/refs.h @@ -336,22 +336,6 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, int refs_pack_refs(struct ref_store *refs, unsigned int flags); /* - * Flags controlling ref_transaction_update(), ref_transaction_create(), etc. - * REF_NODEREF: act on the ref directly, instead of dereferencing - * symbolic references. - * - * Other flags are reserved for internal use. - */ -#define REF_NODEREF 0x01 -#define REF_FORCE_CREATE_REFLOG 0x40 - -/* - * Flags that can be passed in to ref_transaction_update - */ -#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NODEREF | REF_FORCE_CREATE_REFLOG) - -/* * Setup reflog before using. Fill in err and return -1 on failure. */ int refs_create_reflog(struct ref_store *refs, const char *refname, @@ -478,22 +462,23 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); * * refname -- the name of the reference to be affected. * - * new_sha1 -- the SHA-1 that should be set to be the new value of - * the reference. Some functions allow this parameter to be + * new_oid -- the object ID that should be set to be the new value + * of the reference. Some functions allow this parameter to be * NULL, meaning that the reference is not changed, or - * null_sha1, meaning that the reference should be deleted. A + * null_oid, meaning that the reference should be deleted. A * copy of this value is made in the transaction. * - * old_sha1 -- the SHA-1 value that the reference must have before + * old_oid -- the object ID that the reference must have before * the update. Some functions allow this parameter to be NULL, * meaning that the old value of the reference is not checked, - * or null_sha1, meaning that the reference must not exist + * or null_oid, meaning that the reference must not exist * before the update. A copy of this value is made in the * transaction. * * flags -- flags affecting the update, passed to - * update_ref_lock(). Can be REF_NODEREF, which means that - * symbolic references should not be followed. + * update_ref_lock(). Possible flags: REF_NODEREF, + * REF_FORCE_CREATE_REFLOG. See those constants for more + * information. * * msg -- a message describing the change (for the reflog). * @@ -509,11 +494,37 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); */ /* - * Add a reference update to transaction. new_oid is the value that - * the reference should have after the update, or null_oid if it - * should be deleted. If new_oid is NULL, then the reference is not - * changed at all. old_oid is the value that the reference must have - * before the update, or null_oid if it must not have existed + * The following flags can be passed to ref_transaction_update() etc. + * Internally, they are stored in `ref_update::flags`, along with some + * internal flags. + */ + +/* + * Act on the ref directly; i.e., without dereferencing symbolic refs. + * If this flag is not specified, then symbolic references are + * dereferenced and the update is applied to the referent. + */ +#define REF_NODEREF (1 << 0) + +/* + * Force the creation of a reflog for this reference, even if it + * didn't previously have a reflog. + */ +#define REF_FORCE_CREATE_REFLOG (1 << 1) + +/* + * Bitmask of all of the flags that are allowed to be passed in to + * ref_transaction_update() and friends: + */ +#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ + (REF_NODEREF | REF_FORCE_CREATE_REFLOG) + +/* + * Add a reference update to transaction. `new_oid` is the value that + * the reference should have after the update, or `null_oid` if it + * should be deleted. If `new_oid` is NULL, then the reference is not + * changed at all. `old_oid` is the value that the reference must have + * before the update, or `null_oid` if it must not have existed * beforehand. The old value is checked after the lock is taken to * prevent races. If the old value doesn't agree with old_oid, the * whole transaction fails. If old_oid is NULL, then the previous diff --git a/refs/files-backend.c b/refs/files-backend.c index a47771e..bbeafe1 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -10,6 +10,51 @@ #include "../object.h" #include "../dir.h" +/* + * This backend uses the following flags in `ref_update::flags` for + * internal bookkeeping purposes. Their numerical values must not + * conflict with REF_NODEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, + * REF_HAVE_OLD, or REF_ISPRUNING, which are also stored in + * `ref_update::flags`. + */ + +/* + * Used as a flag in ref_update::flags when a loose ref is being + * pruned. This flag must only be used when REF_NODEREF is set. + */ +#define REF_ISPRUNING (1 << 4) + +/* + * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken + * refs (i.e., because the reference is about to be deleted anyway). + */ +#define REF_DELETING (1 << 5) + +/* + * Used as a flag in ref_update::flags when the lockfile needs to be + * committed. + */ +#define REF_NEEDS_COMMIT (1 << 6) + +/* + * Used as a flag in ref_update::flags when we want to log a ref + * update but not actually perform it. This is used when a symbolic + * ref update is split up. + */ +#define REF_LOG_ONLY (1 << 7) + +/* + * Used as a flag in ref_update::flags when the ref_update was via an + * update to HEAD. + */ +#define REF_UPDATE_VIA_HEAD (1 << 8) + +/* + * Used as a flag in ref_update::flags when the loose reference has + * been deleted. + */ +#define REF_DELETED_LOOSE (1 << 9) + struct ref_lock { char *ref_name; struct lock_file lk; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index b0f3e30..2ea02dc 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -8,58 +8,22 @@ */ /* - * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken - * refs (i.e., because the reference is about to be deleted anyway). + * The following flags can appear in `ref_update::flags`. Their + * numerical values must not conflict with those of REF_NODEREF and + * REF_FORCE_CREATE_REFLOG, which are also stored in + * `ref_update::flags`. */ -#define REF_DELETING 0x02 /* - * Used as a flag in ref_update::flags when a loose ref is being - * pruned. This flag must only be used when REF_NODEREF is set. + * The reference should be updated to new_sha1. */ -#define REF_ISPRUNING 0x04 +#define REF_HAVE_NEW (1 << 2) /* - * Used as a flag in ref_update::flags when the reference should be - * updated to new_sha1. + * The current reference's value should be checked to make sure that + * it agrees with old_sha1. */ -#define REF_HAVE_NEW 0x08 - -/* - * Used as a flag in ref_update::flags when old_sha1 should be - * checked. - */ -#define REF_HAVE_OLD 0x10 - -/* - * Used as a flag in ref_update::flags when the lockfile needs to be - * committed. - */ -#define REF_NEEDS_COMMIT 0x20 - -/* - * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a - * value to ref_update::flags - */ - -/* - * Used as a flag in ref_update::flags when we want to log a ref - * update but not actually perform it. This is used when a symbolic - * ref update is split up. - */ -#define REF_LOG_ONLY 0x80 - -/* - * Internal flag, meaning that the containing ref_update was via an - * update to HEAD. - */ -#define REF_UPDATE_VIA_HEAD 0x100 - -/* - * Used as a flag in ref_update::flags when the loose reference has - * been deleted. - */ -#define REF_DELETED_LOOSE 0x200 +#define REF_HAVE_OLD (1 << 3) /* * Return the length of time to retry acquiring a loose reference lock @@ -141,23 +105,22 @@ int copy_reflog_msg(char *buf, const char *msg); * not exist before update. */ struct ref_update { - /* - * If (flags & REF_HAVE_NEW), set the reference to this value: + * If (flags & REF_HAVE_NEW), set the reference to this value + * (or delete it, if `new_oid` is `null_oid`). */ struct object_id new_oid; /* * If (flags & REF_HAVE_OLD), check that the reference - * previously had this value: + * previously had this value (or didn't previously exist, if + * `old_oid` is `null_oid`). */ struct object_id old_oid; /* - * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF, - * REF_DELETING, REF_ISPRUNING, REF_LOG_ONLY, - * REF_UPDATE_VIA_HEAD, REF_NEEDS_COMMIT, and - * REF_DELETED_LOOSE: + * One or more of REF_NODEREF, REF_FORCE_CREATE_REFLOG, + * REF_HAVE_NEW, REF_HAVE_OLD, or backend-specific flags. */ unsigned int flags; -- cgit v0.10.2-6-g49f6 From 91774afcc30c7e8ffdf7b3e587d52c340684364f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:06 +0100 Subject: refs: rename constant `REF_NODEREF` to `REF_NO_DEREF` Even after working with this code for years, I still see this constant name as "ref node ref". Rename it to make it's meaning clearer. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/builtin/am.c b/builtin/am.c index c9bb14a..894290e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2151,7 +2151,7 @@ static void am_abort(struct am_state *state) has_curr_head ? &curr_head : NULL, 0, UPDATE_REFS_DIE_ON_ERR); else if (curr_branch) - delete_ref(NULL, curr_branch, NULL, REF_NODEREF); + delete_ref(NULL, curr_branch, NULL, REF_NO_DEREF); free(curr_branch); am_destroy(state); diff --git a/builtin/branch.c b/builtin/branch.c index b1ed649..33fd5fc 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -258,7 +258,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, } if (delete_ref(NULL, name, is_null_oid(&oid) ? NULL : &oid, - REF_NODEREF)) { + REF_NO_DEREF)) { error(remote_branch ? _("Error deleting remote-tracking branch '%s'") : _("Error deleting branch '%s'"), diff --git a/builtin/checkout.c b/builtin/checkout.c index 463a337..114028e 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -665,7 +665,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts, /* Nothing to do. */ } else if (opts->force_detach || !new->path) { /* No longer on any branch. */ update_ref(msg.buf, "HEAD", &new->commit->object.oid, NULL, - REF_NODEREF, UPDATE_REFS_DIE_ON_ERR); + REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR); if (!opts->quiet) { if (old->path && advice_detached_head && !opts->force_detach) diff --git a/builtin/clone.c b/builtin/clone.c index 695bdd7..557c6c3 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -689,7 +689,7 @@ static void update_head(const struct ref *our, const struct ref *remote, } else if (our) { struct commit *c = lookup_commit_reference(&our->old_oid); /* --branch specifies a non-branch (i.e. tags), detach HEAD */ - update_ref(msg, "HEAD", &c->object.oid, NULL, REF_NODEREF, + update_ref(msg, "HEAD", &c->object.oid, NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR); } else if (remote) { /* @@ -697,7 +697,7 @@ static void update_head(const struct ref *our, const struct ref *remote, * HEAD points to a branch but we don't know which one. * Detach HEAD in all these cases. */ - update_ref(msg, "HEAD", &remote->old_oid, NULL, REF_NODEREF, + update_ref(msg, "HEAD", &remote->old_oid, NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR); } } diff --git a/builtin/notes.c b/builtin/notes.c index 12afdf1..d7754db 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -686,7 +686,7 @@ static int merge_abort(struct notes_merge_options *o) if (delete_ref(NULL, "NOTES_MERGE_PARTIAL", NULL, 0)) ret += error(_("failed to delete ref NOTES_MERGE_PARTIAL")); - if (delete_ref(NULL, "NOTES_MERGE_REF", NULL, REF_NODEREF)) + if (delete_ref(NULL, "NOTES_MERGE_REF", NULL, REF_NO_DEREF)) ret += error(_("failed to delete ref NOTES_MERGE_REF")); if (notes_merge_abort(o)) ret += error(_("failed to remove 'git notes merge' worktree")); diff --git a/builtin/remote.c b/builtin/remote.c index 0fddc64..3d38c61 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -693,7 +693,7 @@ static int mv(int argc, const char **argv) read_ref_full(item->string, RESOLVE_REF_READING, &oid, &flag); if (!(flag & REF_ISSYMREF)) continue; - if (delete_ref(NULL, item->string, NULL, REF_NODEREF)) + if (delete_ref(NULL, item->string, NULL, REF_NO_DEREF)) die(_("deleting '%s' failed"), item->string); } for (i = 0; i < remote_branches.nr; i++) { @@ -788,7 +788,7 @@ static int rm(int argc, const char **argv) strbuf_release(&buf); if (!result) - result = delete_refs("remote: remove", &branches, REF_NODEREF); + result = delete_refs("remote: remove", &branches, REF_NO_DEREF); string_list_clear(&branches, 0); if (skipped.nr) { @@ -1255,7 +1255,7 @@ static int set_head(int argc, const char **argv) head_name = xstrdup(states.heads.items[0].string); free_remote_ref_states(&states); } else if (opt_d && !opt_a && argc == 1) { - if (delete_ref(NULL, buf.buf, NULL, REF_NODEREF)) + if (delete_ref(NULL, buf.buf, NULL, REF_NO_DEREF)) result |= error(_("Could not delete %s"), buf.buf); } else usage_with_options(builtin_remote_sethead_usage, options); diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c index 17aabaa..80237f0 100644 --- a/builtin/symbolic-ref.c +++ b/builtin/symbolic-ref.c @@ -58,7 +58,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix) die("Cannot delete %s, not a symbolic ref", argv[0]); if (!strcmp(argv[0], "HEAD")) die("deleting '%s' is not allowed", argv[0]); - return delete_ref(NULL, argv[0], NULL, REF_NODEREF); + return delete_ref(NULL, argv[0], NULL, REF_NO_DEREF); } switch (argc) { diff --git a/builtin/update-ref.c b/builtin/update-ref.c index cf1552b..4b4714b 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -312,7 +312,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction, static const char *parse_cmd_option(struct strbuf *input, const char *next) { if (!strncmp(next, "no-deref", 8) && next[8] == line_termination) - update_flags |= REF_NODEREF; + update_flags |= REF_NO_DEREF; else die("option unknown: %s", next); return next + 8; @@ -427,7 +427,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) } if (no_deref) - flags = REF_NODEREF; + flags = REF_NO_DEREF; if (delete) /* * For purposes of backwards compatibility, we treat diff --git a/refs.h b/refs.h index 261d46c..d396012 100644 --- a/refs.h +++ b/refs.h @@ -476,7 +476,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); * transaction. * * flags -- flags affecting the update, passed to - * update_ref_lock(). Possible flags: REF_NODEREF, + * update_ref_lock(). Possible flags: REF_NO_DEREF, * REF_FORCE_CREATE_REFLOG. See those constants for more * information. * @@ -504,7 +504,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); * If this flag is not specified, then symbolic references are * dereferenced and the update is applied to the referent. */ -#define REF_NODEREF (1 << 0) +#define REF_NO_DEREF (1 << 0) /* * Force the creation of a reflog for this reference, even if it @@ -517,7 +517,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); * ref_transaction_update() and friends: */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NODEREF | REF_FORCE_CREATE_REFLOG) + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG) /* * Add a reference update to transaction. `new_oid` is the value that diff --git a/refs/files-backend.c b/refs/files-backend.c index bbeafe1..71e088e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -13,14 +13,14 @@ /* * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not - * conflict with REF_NODEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, + * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, * REF_HAVE_OLD, or REF_ISPRUNING, which are also stored in * `ref_update::flags`. */ /* * Used as a flag in ref_update::flags when a loose ref is being - * pruned. This flag must only be used when REF_NODEREF is set. + * pruned. This flag must only be used when REF_NO_DEREF is set. */ #define REF_ISPRUNING (1 << 4) @@ -1044,7 +1044,7 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r) goto cleanup; ref_transaction_add_update( transaction, r->name, - REF_NODEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_ISPRUNING, + REF_NO_DEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_ISPRUNING, &null_oid, &r->oid, NULL); if (ref_transaction_commit(transaction, &err)) goto cleanup; @@ -1133,7 +1133,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags) */ if (ref_transaction_update(transaction, iter->refname, iter->oid, NULL, - REF_NODEREF, NULL, &err)) + REF_NO_DEREF, NULL, &err)) die("failure preparing to create packed reference %s: %s", iter->refname, err.buf); @@ -1336,7 +1336,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, } if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname, - &orig_oid, REF_NODEREF)) { + &orig_oid, REF_NO_DEREF)) { error("unable to delete old %s", oldrefname); goto rollback; } @@ -1352,7 +1352,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, &oid, NULL) && refs_delete_ref(&refs->base, NULL, newrefname, - NULL, REF_NODEREF)) { + NULL, REF_NO_DEREF)) { if (errno == EISDIR) { struct strbuf path = STRBUF_INIT; int result; @@ -1377,7 +1377,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, logmoved = log; lock = lock_ref_oid_basic(refs, newrefname, NULL, NULL, NULL, - REF_NODEREF, NULL, &err); + REF_NO_DEREF, NULL, &err); if (!lock) { if (copy) error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf); @@ -1400,7 +1400,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store, rollback: lock = lock_ref_oid_basic(refs, oldrefname, NULL, NULL, NULL, - REF_NODEREF, NULL, &err); + REF_NO_DEREF, NULL, &err); if (!lock) { error("unable to lock %s for rollback: %s", oldrefname, err.buf); strbuf_release(&err); @@ -1816,7 +1816,7 @@ static int files_create_symref(struct ref_store *ref_store, int ret; lock = lock_ref_oid_basic(refs, refname, NULL, - NULL, NULL, REF_NODEREF, NULL, + NULL, NULL, REF_NO_DEREF, NULL, &err); if (!lock) { error("%s", err.buf); @@ -2200,7 +2200,7 @@ static int split_head_update(struct ref_update *update, new_update = ref_transaction_add_update( transaction, "HEAD", - update->flags | REF_LOG_ONLY | REF_NODEREF, + update->flags | REF_LOG_ONLY | REF_NO_DEREF, &update->new_oid, &update->old_oid, update->msg); @@ -2219,8 +2219,8 @@ static int split_head_update(struct ref_update *update, /* * update is for a symref that points at referent and doesn't have - * REF_NODEREF set. Split it into two updates: - * - The original update, but with REF_LOG_ONLY and REF_NODEREF set + * REF_NO_DEREF set. Split it into two updates: + * - The original update, but with REF_LOG_ONLY and REF_NO_DEREF set * - A new, separate update for the referent reference * Note that the new update will itself be subject to splitting when * the iteration gets to it. @@ -2275,7 +2275,7 @@ static int split_symref_update(struct files_ref_store *refs, * doesn't need to check its old SHA-1 value, as that will be * done when new_update is processed. */ - update->flags |= REF_LOG_ONLY | REF_NODEREF; + update->flags |= REF_LOG_ONLY | REF_NO_DEREF; update->flags &= ~REF_HAVE_OLD; /* @@ -2344,7 +2344,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid, * - Check that its old SHA-1 value (if specified) is correct, and in * any case record it in update->lock->old_oid for later use when * writing the reflog. - * - If it is a symref update without REF_NODEREF, split it up into a + * - If it is a symref update without REF_NO_DEREF, split it up into a * REF_LOG_ONLY update of the symref and add a separate update for * the referent to transaction. * - If it is an update of head_ref, add a corresponding REF_LOG_ONLY @@ -2392,7 +2392,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, update->backend_data = lock; if (update->type & REF_ISSYMREF) { - if (update->flags & REF_NODEREF) { + if (update->flags & REF_NO_DEREF) { /* * We won't be reading the referent as part of * the transaction, so we have to read it here @@ -2564,7 +2564,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, * split_symref_update() or split_head_update(), those * functions will check that the new updates don't have the * same refname as any existing ones.) Also fail if any of the - * updates use REF_ISPRUNING without REF_NODEREF. + * updates use REF_ISPRUNING without REF_NO_DEREF. */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; @@ -2572,8 +2572,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, string_list_append(&affected_refnames, update->refname); if ((update->flags & REF_ISPRUNING) && - !(update->flags & REF_NODEREF)) - BUG("REF_ISPRUNING set without REF_NODEREF"); + !(update->flags & REF_NO_DEREF)) + BUG("REF_ISPRUNING set without REF_NO_DEREF"); /* * We store a pointer to update in item->util, but at @@ -2651,7 +2651,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, ref_transaction_add_update( packed_transaction, update->refname, - REF_HAVE_NEW | REF_NODEREF, + REF_HAVE_NEW | REF_NO_DEREF, &update->new_oid, NULL, NULL); } @@ -2995,7 +2995,7 @@ static int files_reflog_expire(struct ref_store *ref_store, * reference if --updateref was specified: */ lock = lock_ref_oid_basic(refs, refname, oid, - NULL, NULL, REF_NODEREF, + NULL, NULL, REF_NO_DEREF, &type, &err); if (!lock) { error("cannot lock ref '%s': %s", refname, err.buf); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2ea02dc..f9c6e72 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -9,7 +9,7 @@ /* * The following flags can appear in `ref_update::flags`. Their - * numerical values must not conflict with those of REF_NODEREF and + * numerical values must not conflict with those of REF_NO_DEREF and * REF_FORCE_CREATE_REFLOG, which are also stored in * `ref_update::flags`. */ @@ -119,7 +119,7 @@ struct ref_update { struct object_id old_oid; /* - * One or more of REF_NODEREF, REF_FORCE_CREATE_REFLOG, + * One or more of REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, * REF_HAVE_NEW, REF_HAVE_OLD, or backend-specific flags. */ unsigned int flags; diff --git a/sequencer.c b/sequencer.c index 64abaad..3b88ab2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1116,11 +1116,11 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, */ if (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) && update_ref(NULL, "CHERRY_PICK_HEAD", &commit->object.oid, NULL, - REF_NODEREF, UPDATE_REFS_MSG_ON_ERR)) + REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) res = -1; if (command == TODO_REVERT && ((opts->no_commit && res == 0) || res == 1) && update_ref(NULL, "REVERT_HEAD", &commit->object.oid, NULL, - REF_NODEREF, UPDATE_REFS_MSG_ON_ERR)) + REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) res = -1; if (res) { @@ -2125,7 +2125,7 @@ cleanup_head_ref: msg = reflog_message(opts, "finish", "%s onto %s", head_ref.buf, buf.buf); if (update_ref(msg, head_ref.buf, &head, &orig, - REF_NODEREF, UPDATE_REFS_MSG_ON_ERR)) { + REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) { res = error(_("could not update %s"), head_ref.buf); goto cleanup_head_ref; -- cgit v0.10.2-6-g49f6 From acedcde76db257e4822291a180d5985852f8e621 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:07 +0100 Subject: refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING` Underscores are cheap, and help readability. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs/files-backend.c b/refs/files-backend.c index 71e088e..bb10b71 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -14,7 +14,7 @@ * This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, - * REF_HAVE_OLD, or REF_ISPRUNING, which are also stored in + * REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in * `ref_update::flags`. */ @@ -22,7 +22,7 @@ * Used as a flag in ref_update::flags when a loose ref is being * pruned. This flag must only be used when REF_NO_DEREF is set. */ -#define REF_ISPRUNING (1 << 4) +#define REF_IS_PRUNING (1 << 4) /* * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken @@ -1044,7 +1044,7 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r) goto cleanup; ref_transaction_add_update( transaction, r->name, - REF_NO_DEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_ISPRUNING, + REF_NO_DEREF | REF_HAVE_NEW | REF_HAVE_OLD | REF_IS_PRUNING, &null_oid, &r->oid, NULL); if (ref_transaction_commit(transaction, &err)) goto cleanup; @@ -2177,7 +2177,7 @@ static int split_head_update(struct ref_update *update, struct ref_update *new_update; if ((update->flags & REF_LOG_ONLY) || - (update->flags & REF_ISPRUNING) || + (update->flags & REF_IS_PRUNING) || (update->flags & REF_UPDATE_VIA_HEAD)) return 0; @@ -2564,16 +2564,16 @@ static int files_transaction_prepare(struct ref_store *ref_store, * split_symref_update() or split_head_update(), those * functions will check that the new updates don't have the * same refname as any existing ones.) Also fail if any of the - * updates use REF_ISPRUNING without REF_NO_DEREF. + * updates use REF_IS_PRUNING without REF_NO_DEREF. */ for (i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; struct string_list_item *item = string_list_append(&affected_refnames, update->refname); - if ((update->flags & REF_ISPRUNING) && + if ((update->flags & REF_IS_PRUNING) && !(update->flags & REF_NO_DEREF)) - BUG("REF_ISPRUNING set without REF_NO_DEREF"); + BUG("REF_IS_PRUNING set without REF_NO_DEREF"); /* * We store a pointer to update in item->util, but at @@ -2632,7 +2632,7 @@ static int files_transaction_prepare(struct ref_store *ref_store, if (update->flags & REF_DELETING && !(update->flags & REF_LOG_ONLY) && - !(update->flags & REF_ISPRUNING)) { + !(update->flags & REF_IS_PRUNING)) { /* * This reference has to be deleted from * packed-refs if it exists there. @@ -2749,7 +2749,7 @@ static int files_transaction_finish(struct ref_store *ref_store, struct ref_update *update = transaction->updates[i]; if (update->flags & REF_DELETING && !(update->flags & REF_LOG_ONLY) && - !(update->flags & REF_ISPRUNING)) { + !(update->flags & REF_IS_PRUNING)) { strbuf_reset(&sb); files_reflog_path(refs, &sb, update->refname); if (!unlink_or_warn(sb.buf)) -- cgit v0.10.2-6-g49f6 From 417018826285140f9e948649ff9cf75a193075d2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:08 +0100 Subject: write_packed_entry(): take `object_id` arguments Change `write_packed_entry()` to take `struct object_id *` rather than `unsigned char *` arguments. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 74f1dea..43ad74f 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -961,11 +961,11 @@ static struct ref_iterator *packed_ref_iterator_begin( * by the failing call to `fprintf()`. */ static int write_packed_entry(FILE *fh, const char *refname, - const unsigned char *sha1, - const unsigned char *peeled) + const struct object_id *oid, + const struct object_id *peeled) { - if (fprintf(fh, "%s %s\n", sha1_to_hex(sha1), refname) < 0 || - (peeled && fprintf(fh, "^%s\n", sha1_to_hex(peeled)) < 0)) + if (fprintf(fh, "%s %s\n", oid_to_hex(oid), refname) < 0 || + (peeled && fprintf(fh, "^%s\n", oid_to_hex(peeled)) < 0)) return -1; return 0; @@ -1203,8 +1203,8 @@ static int write_with_updates(struct packed_ref_store *refs, int peel_error = ref_iterator_peel(iter, &peeled); if (write_packed_entry(out, iter->refname, - iter->oid->hash, - peel_error ? NULL : peeled.hash)) + iter->oid, + peel_error ? NULL : &peeled)) goto write_error; if ((ok = ref_iterator_advance(iter)) != ITER_OK) @@ -1224,8 +1224,8 @@ static int write_with_updates(struct packed_ref_store *refs, &peeled); if (write_packed_entry(out, update->refname, - update->new_oid.hash, - peel_error ? NULL : peeled.hash)) + &update->new_oid, + peel_error ? NULL : &peeled)) goto write_error; i++; -- cgit v0.10.2-6-g49f6 From 78fb457968591887ebb331d3d0475c00c7dbb317 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 5 Nov 2017 09:42:09 +0100 Subject: refs: update some more docs to use "oid" rather than "sha1" Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano diff --git a/refs.c b/refs.c index 0d9a134..339d431 100644 --- a/refs.c +++ b/refs.c @@ -770,7 +770,7 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid, if (cb->cutoff_cnt) *cb->cutoff_cnt = cb->reccnt - 1; /* - * we have not yet updated cb->[n|o]sha1 so they still + * we have not yet updated cb->[n|o]oid so they still * hold the values for the previous record. */ if (!is_null_oid(&cb->ooid)) { diff --git a/refs.h b/refs.h index d396012..18582a4 100644 --- a/refs.h +++ b/refs.h @@ -126,7 +126,7 @@ int peel_ref(const char *refname, struct object_id *oid); /** * Resolve refname in the nested "gitlink" repository in the specified * submodule (which must be non-NULL). If the resolution is - * successful, return 0 and set sha1 to the name of the object; + * successful, return 0 and set oid to the name of the object; * otherwise, return a non-zero value. */ int resolve_gitlink_ref(const char *submodule, const char *refname, @@ -260,7 +260,7 @@ struct ref_transaction; /* * The signature for the callback function for the for_each_*() - * functions below. The memory pointed to by the refname and sha1 + * functions below. The memory pointed to by the refname and oid * arguments is only guaranteed to be valid for the duration of a * single callback invocation. */ @@ -354,7 +354,7 @@ int reflog_exists(const char *refname); /* * Delete the specified reference. If old_oid is non-NULL, then - * verify that the current value of the reference is old_sha1 before + * verify that the current value of the reference is old_oid before * deleting it. If old_oid is NULL, delete the reference if it * exists, regardless of its old value. It is an error for old_oid to * be null_oid. msg and flags are passed through to @@ -633,7 +633,7 @@ int ref_transaction_abort(struct ref_transaction *transaction, * It is a bug to call this function when there might be other * processes accessing the repository or if there are existing * references that might conflict with the ones being created. All - * old_sha1 values must either be absent or NULL_SHA1. + * old_oid values must either be absent or null_oid. */ int initial_ref_transaction_commit(struct ref_transaction *transaction, struct strbuf *err); diff --git a/refs/files-backend.c b/refs/files-backend.c index bb10b71..2298f90 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -240,7 +240,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store, } else if (is_null_oid(&oid)) { /* * It is so astronomically unlikely - * that NULL_SHA1 is the SHA-1 of an + * that null_oid is the OID of an * actual object that we consider its * appearance in a loose reference * file to be repo corruption @@ -473,7 +473,7 @@ static void unlock_ref(struct ref_lock *lock) * are passed to refs_verify_refname_available() for this check. * * If mustexist is not set and the reference is not found or is - * broken, lock the reference anyway but clear sha1. + * broken, lock the reference anyway but clear old_oid. * * Return 0 on success. On failure, write an error message to err and * return TRANSACTION_NAME_CONFLICT or TRANSACTION_GENERIC_ERROR. @@ -1648,9 +1648,8 @@ static int files_log_ref_write(struct files_ref_store *refs, } /* - * Write sha1 into the open lockfile, then close the lockfile. On - * errors, rollback the lockfile, fill in *err and - * return -1. + * Write oid into the open lockfile, then close the lockfile. On + * errors, rollback the lockfile, fill in *err and return -1. */ static int write_ref_to_lockfile(struct ref_lock *lock, const struct object_id *oid, struct strbuf *err) @@ -2272,7 +2271,7 @@ static int split_symref_update(struct files_ref_store *refs, /* * Change the symbolic ref update to log only. Also, it - * doesn't need to check its old SHA-1 value, as that will be + * doesn't need to check its old OID value, as that will be * done when new_update is processed. */ update->flags |= REF_LOG_ONLY | REF_NO_DEREF; @@ -2341,7 +2340,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid, * Prepare for carrying out update: * - Lock the reference referred to by update. * - Read the reference under lock. - * - Check that its old SHA-1 value (if specified) is correct, and in + * - Check that its old OID value (if specified) is correct, and in * any case record it in update->lock->old_oid for later use when * writing the reflog. * - If it is a symref update without REF_NO_DEREF, split it up into a @@ -2396,7 +2395,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, /* * We won't be reading the referent as part of * the transaction, so we have to read it here - * to record and possibly check old_sha1: + * to record and possibly check old_oid: */ if (refs_read_ref_full(&refs->base, referent.buf, 0, @@ -2416,7 +2415,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, /* * Create a new update for the reference this * symref is pointing at. Also, we will record - * and verify old_sha1 for this update as part + * and verify old_oid for this update as part * of processing the split-off update, so we * don't have to do it here. */ @@ -2436,7 +2435,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, /* * If this update is happening indirectly because of a - * symref update, record the old SHA-1 in the parent + * symref update, record the old OID in the parent * update: */ for (parent_update = update->parent_update; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 43ad74f..72164a1 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -744,7 +744,7 @@ static int packed_read_raw_ref(struct ref_store *ref_store, /* * This value is set in `base.flags` if the peeled value of the * current reference is known. In that case, `peeled` contains the - * correct peeled value for the reference, which might be `null_sha1` + * correct peeled value for the reference, which might be `null_oid` * if the reference is not a tag or if it is broken. */ #define REF_KNOWS_PEELED 0x40 diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 043eb83..82c1cf9 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -260,8 +260,8 @@ int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref) /* * Emit a warning and return true iff ref1 and ref2 have the same name - * and the same sha1. Die if they have the same name but different - * sha1s. + * and the same oid. Die if they have the same name but different + * oids. */ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2) { diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f9c6e72..dd83431 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -15,13 +15,13 @@ */ /* - * The reference should be updated to new_sha1. + * The reference should be updated to new_oid. */ #define REF_HAVE_NEW (1 << 2) /* * The current reference's value should be checked to make sure that - * it agrees with old_sha1. + * it agrees with old_oid. */ #define REF_HAVE_OLD (1 << 3) @@ -86,7 +86,7 @@ enum peel_status { * tag recursively until a non-tag is found. If successful, store the * result to oid and return PEEL_PEELED. If the object is not a tag * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively, - * and leave sha1 unchanged. + * and leave oid unchanged. */ enum peel_status peel_object(const struct object_id *name, struct object_id *oid); @@ -98,11 +98,11 @@ enum peel_status peel_object(const struct object_id *name, struct object_id *oid int copy_reflog_msg(char *buf, const char *msg); /** - * Information needed for a single ref update. Set new_sha1 to the new - * value or to null_sha1 to delete the ref. To check the old value - * while the ref is locked, set (flags & REF_HAVE_OLD) and set - * old_sha1 to the old value, or to null_sha1 to ensure the ref does - * not exist before update. + * Information needed for a single ref update. Set new_oid to the new + * value or to null_oid to delete the ref. To check the old value + * while the ref is locked, set (flags & REF_HAVE_OLD) and set old_oid + * to the old value, or to null_oid to ensure the ref does not exist + * before update. */ struct ref_update { /* @@ -158,7 +158,7 @@ int ref_update_reject_duplicates(struct string_list *refnames, /* * Add a ref_update with the specified properties to transaction, and * return a pointer to the new object. This function does not verify - * that refname is well-formed. new_sha1 and old_sha1 are only + * that refname is well-formed. new_oid and old_oid are only * dereferenced if the REF_HAVE_NEW and REF_HAVE_OLD bits, * respectively, are set in flags. */ -- cgit v0.10.2-6-g49f6