Re: [PATCH v3 05/44] refs.c: move update_ref to refs.c
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Oct 13, 2015, 03:41 UTC
- Message-ID
- <561C7D7B.8020807@alum.mit.edu>
- In-Reply-To
- <1444686725-27660-6-git-send-email-dturner@twopensource.com>
On 10/12/2015 11:51 PM, David Turner wrote:
Show 95 quoted lines
> From: Ronnie Sahlberg <sahlberg@google.com>
>
> Move update_ref() to the refs.c file since this function does not
> contain any backend specific code. Move the ref classifier functions
> as well, since update_ref depends on them.
>
> Based on Ronnie Sahlberg's patch
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> refs-be-files.c | 117 +-------------------------------------------------------
> refs.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+), 116 deletions(-)
>
> diff --git a/refs-be-files.c b/refs-be-files.c
> index d0dfdfc..7fe4931 100644
> --- a/refs-be-files.c
> +++ b/refs-be-files.c
> [...]
> @@ -2879,77 +2877,6 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
> [...]
> -static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
> - const unsigned char *old_sha1, struct strbuf *err)
> -{
> - const char *filename;
> - int fd;
> - static struct lock_file lock;
> - struct strbuf buf = STRBUF_INIT;
> - int ret = -1;
> -
> - strbuf_addf(&buf, "%s\n", sha1_to_hex(sha1));
> -
> - filename = git_path("%s", pseudoref);
> - fd = hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
> - if (fd < 0) {
> - strbuf_addf(err, "Could not open '%s' for writing: %s",
> - filename, strerror(errno));
> - return -1;
> - }
> -
> - if (old_sha1) {
> - unsigned char actual_old_sha1[20];
> -
> - if (read_ref(pseudoref, actual_old_sha1))
> - die("could not read ref '%s'", pseudoref);
> - if (hashcmp(actual_old_sha1, old_sha1)) {
> - strbuf_addf(err, "Unexpected sha1 when writing %s", pseudoref);
> - rollback_lock_file(&lock);
> - goto done;
> - }
> - }
> -
> - if (write_in_full(fd, buf.buf, buf.len) != buf.len) {
> - strbuf_addf(err, "Could not write to '%s'", filename);
> - rollback_lock_file(&lock);
> - goto done;
> - }
> -
> - commit_lock_file(&lock);
> - ret = 0;
> -done:
> - strbuf_release(&buf);
> - return ret;
> -}
> -
> [...]
> diff --git a/refs.c b/refs.c
> index 77492ff..2d10708 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1,3 +1,118 @@
> [...]
> +static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
> + const unsigned char *old_sha1, struct strbuf *err)
> +{
> + const char *filename;
> + int fd;
> + static struct lock_file lock;
> + struct strbuf buf = STRBUF_INIT;
> + int ret = -1;
> +
> + strbuf_addf(&buf, "%s\n", sha1_to_hex(sha1));
> +
> + filename = git_path("%s", pseudoref);
> + fd = hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
> + if (fd < 0) {
> + strbuf_addf(err, "Could not open '%s' for writing: %s",
> + filename, strerror(errno));
> + return -1;
> + }
> +
> + if (old_sha1) {
> + unsigned char actual_old_sha1[20];
> + read_ref(pseudoref, actual_old_sha1);The original read
if (read_ref(pseudoref, actual_old_sha1))
die("could not read ref '%s'", pseudoref);This seems like an important test. What happened to it?
If its removal was intentional, it deserves a careful explanation (and should probably be done as a separate commit). If it was an accident, please consider how this accident arose and try to think about whether similar accidents might have happened elsewhere in this series.
> [...]
Michael
-- Michael Haggerty mhagger@alum.mit.edu