git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/8] Convert struct ref to use object_id.

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 11, 2015, 15:41 UTC
Message-ID
<5579AC1D.6000606@alum.mit.edu>
In-Reply-To
<1433867316-663554-4-git-send-email-sandals@crustytoothpaste.net>
I visually inspected patches 1 and 2 without finding any problems.

Regarding this patch, I saw a few functions where you could convert local variables to "struct object_id" and then change function calls like hashcpy() to oidcpy(). See below. I'm not sure if it makes sense to do that in this same patch.

For that matter, it seems to me that it should be possible to change *all* local

    unsigned char $variable[20];
to
    struct object_id $variable;

without having to change any external interfaces. I wonder whether it would be advisable to make that change early in this transition? It would have the advantage that during later refactorings, where, e.g., a function needs a "struct object_id *" parameter, you would often already have one handy.

Michael
On 06/09/2015 06:28 PM, brian m. carlson wrote:
Show 39 quoted lines
> Use struct object_id in three fields in struct ref and convert all the
> necessary places that use it.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/clone.c        | 16 +++++++-------
>  builtin/fetch-pack.c   |  4 ++--
>  builtin/fetch.c        | 50 +++++++++++++++++++++----------------------
>  builtin/ls-remote.c    |  2 +-
>  builtin/receive-pack.c |  2 +-
>  builtin/remote.c       | 12 +++++------
>  connect.c              |  2 +-
>  fetch-pack.c           | 18 ++++++++--------
>  http-push.c            | 46 +++++++++++++++++++--------------------
>  http.c                 |  2 +-
>  remote-curl.c          | 10 ++++-----
>  remote.c               | 58 +++++++++++++++++++++++++-------------------------
>  remote.h               |  6 +++---
>  send-pack.c            | 16 +++++++-------
>  transport-helper.c     | 18 ++++++++--------
>  transport.c            | 32 ++++++++++++++--------------
>  transport.h            |  8 +++----
>  walker.c               |  2 +-
>  18 files changed, 152 insertions(+), 152 deletions(-)
> 
> [...]
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 4a6b340..19215b3 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -17,7 +17,7 @@ static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc,
>  	unsigned char sha1[20];
>  
>  	if (namelen > 41 && name[40] == ' ' && !get_sha1_hex(name, sha1)) {
> -		hashcpy(ref->old_sha1, sha1);
> +		hashcpy(ref->old_oid.hash, sha1);
>  		name += 41;
>  		namelen -= 41;
>  	}
Variable "sha1" in this function could become "struct object_id".
Show 14 quoted lines
> [...]
> diff --git a/connect.c b/connect.c
> index c0144d8..f8b10eb 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -165,7 +165,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  		if (!check_ref(name, flags))
>  			continue;
>  		ref = alloc_ref(buffer + 41);
> -		hashcpy(ref->old_sha1, old_sha1);
> +		hashcpy(ref->old_oid.hash, old_sha1);
>  		*list = ref;
>  		list = &ref->next;
>  		got_at_least_one_head = 1;

old_sha1 in this function could become "struct object_id". Also, this function has a few "20" and "41" and "42" hard-coded literals.

Show 15 quoted lines
> [...]
> diff --git a/remote-curl.c b/remote-curl.c
> index af7b678..80cb4c7 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> [...]
> @@ -814,7 +814,7 @@ static void parse_fetch(struct strbuf *buf)
>  				die("protocol error: expected sha/ref, got %s'", p);
>  
>  			ref = alloc_ref(name);
> -			hashcpy(ref->old_sha1, old_sha1);
> +			hashcpy(ref->old_oid.hash, old_sha1);
>  
>  			*list = ref;
>  			list = &ref->next;
old_sha1 in this code block could be converted to "struct object_id".
Show 14 quoted lines
> diff --git a/remote.c b/remote.c
> index 26504b7..706d2fb 100644
> --- a/remote.c
> +++ b/remote.c
> [...]
> @@ -1131,7 +1131,7 @@ static int try_explicit_object_name(const char *name,
>  
>  	if (match) {
>  		*match = alloc_ref(name);
> -		hashcpy((*match)->new_sha1, sha1);
> +		hashcpy((*match)->new_oid.hash, sha1);
>  	}
>  	return 0;
>  }
The "sha1" variable in this function could become a "struct object_id".
Show 7 quoted lines
> [...]
> @@ -2181,7 +2181,7 @@ static int one_local_ref(const char *refname, const struct object_id *oid,
>  
>  	len = strlen(refname) + 1;
>  	ref = xcalloc(1, sizeof(*ref) + len);
> -	hashcpy(ref->new_sha1, oid->hash);
> +	hashcpy(ref->new_oid.hash, oid->hash);
This could become oidcopy().
Show 18 quoted lines
>  	memcpy(ref->name, refname, len);
>  	**local_tail = ref;
>  	*local_tail = &ref->next;
> [...]
> diff --git a/transport-helper.c b/transport-helper.c
> index 5d99a6b..4ca3e80 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> [...]
> @@ -883,7 +883,7 @@ static int push_refs_with_export(struct transport *transport,
>  		if (private && !get_sha1(private, sha1)) {
>  			strbuf_addf(&buf, "^%s", private);
>  			string_list_append(&revlist_args, strbuf_detach(&buf, NULL));
> -			hashcpy(ref->old_sha1, sha1);
> +			hashcpy(ref->old_oid.hash, sha1);
>  		}
>  		free(private);
>  
sha1 in this function could become "struct object_id".
> [...]
Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
Previous: brian m. carlsonNext: brian m. carlson
Message 7 of 23 in “object_id part 2”
  1. 0/8 object_id part 2brian m. carlson, Jun 9, 2015
  2. 1/8 refs: convert some internal functions to use object_idbrian m. carlson, Jun 9, 2015
  3. 2/8 sha1_file: introduce has_object_file helper.brian m. carlson, Jun 9, 2015
  4. Duy NguyenJun 10, 2015
  5. brian m. carlsonJun 10, 2015
  6. 3/8 Convert struct ref to use object_id.brian m. carlson, Jun 9, 2015
  7. Michael HaggertyJun 11, 2015
  8. 4/8 Add a utility function to make parsing hex values easier.brian m. carlson, Jun 9, 2015
  9. Michael HaggertyJun 11, 2015
  10. 5/8 add_sought_entry_mem: convert to struct object_idbrian m. carlson, Jun 9, 2015
  11. 6/8 parse_fetch: convert to use struct object_idbrian m. carlson, Jun 9, 2015
  12. 7/8 ref_newer: convert to use struct object_idbrian m. carlson, Jun 9, 2015
  13. Junio C HamanoJun 10, 2015
  14. brian m. carlsonJun 10, 2015
  15. brian m. carlsonJun 11, 2015
  16. Junio C HamanoJun 11, 2015
  17. brian m. carlsonJun 11, 2015
  18. Junio C HamanoJun 11, 2015
  19. brian m. carlsonJun 12, 2015
  20. Junio C HamanoJun 12, 2015
  21. brian m. carlsonJun 12, 2015
  22. Michael HaggertyJun 13, 2015
  23. brian m. carlsonJun 13, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.