Re: [PATCH 8/9] cache-tree: convert struct cache_tree to use object_id

2014-05-06 Thread Michael Haggerty
On 05/03/2014 10:12 PM, brian m. carlson wrote:
> [...]
> diff --git a/cache-tree.c b/cache-tree.c
> index 7fa524a..b7b2d06 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c

In this file I also found a couple other "20" that could be converted to
GIT_OID_RAWSZ:

Around line 369:
strbuf_add(&buffer, sha1, 20);

And around line 490 (three instances):
if (size < 20)
goto free_return;
hashcpy(it->sha1, (const unsigned char*)buf);
buf += 20;
size -= 20;

I guess a search for "\<[24][0-9]\>" will find most (but not all!) of
the literal constants that are derived from GIT_OID_RAWSZ and GIT_OID_HEXSZ.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 8/9] cache-tree: convert struct cache_tree to use object_id

2014-05-06 Thread Michael Haggerty
On 05/03/2014 10:12 PM, brian m. carlson wrote:
> Signed-off-by: brian m. carlson 
> ---
>  builtin/commit.c   |  2 +-
>  builtin/fsck.c |  4 ++--
>  cache-tree.c   | 30 +++---
>  cache-tree.h   |  3 ++-
>  merge-recursive.c  |  2 +-
>  reachable.c|  2 +-
>  sequencer.c|  2 +-
>  test-dump-cache-tree.c |  4 ++--
>  8 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 9cfef6c..639f843 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char 
> *prefix)
>   append_merge_tag_headers(parents, &tail);
>   }
>  
> - if (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1,
> + if (commit_tree_extended(&sb, active_cache_tree->sha1.oid, parents, 
> sha1,
>author_ident.buf, sign_commit, extra)) {
>   rollback_index_files();
>   die(_("failed to write commit object"));
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index fc150c8..6854c81 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -587,10 +587,10 @@ static int fsck_cache_tree(struct cache_tree *it)
>   fprintf(stderr, "Checking cache tree\n");
>  
>   if (0 <= it->entry_count) {
> - struct object *obj = parse_object(it->sha1);
> + struct object *obj = parse_object(it->sha1.oid);
>   if (!obj) {
>   error("%s: invalid sha1 pointer in cache-tree",
> -   sha1_to_hex(it->sha1));
> +   sha1_to_hex(it->sha1.oid));
>   return 1;
>   }
>   obj->used = 1;
> diff --git a/cache-tree.c b/cache-tree.c
> index 7fa524a..b7b2d06 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -219,7 +219,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
>   int i;
>   if (!it)
>   return 0;
> - if (it->entry_count < 0 || !has_sha1_file(it->sha1))
> + if (it->entry_count < 0 || !has_sha1_file(it->sha1.oid))
>   return 0;
>   for (i = 0; i < it->subtree_nr; i++) {
>   if (!cache_tree_fully_valid(it->down[i]->cache_tree))
> @@ -244,7 +244,7 @@ static int update_one(struct cache_tree *it,
>  
>   *skip_count = 0;
>  
> - if (0 <= it->entry_count && has_sha1_file(it->sha1))
> + if (0 <= it->entry_count && has_sha1_file(it->sha1.oid))
>   return it->entry_count;
>  
>   /*
> @@ -311,7 +311,7 @@ static int update_one(struct cache_tree *it,
>   struct cache_tree_sub *sub;
>   const char *path, *slash;
>   int pathlen, entlen;
> - const unsigned char *sha1;
> + const struct object_id *sha1;
>   unsigned mode;
>  
>   path = ce->name;
> @@ -327,21 +327,21 @@ static int update_one(struct cache_tree *it,
>   die("cache-tree.c: '%.*s' in '%s' not found",
>   entlen, path + baselen, path);
>   i += sub->count;
> - sha1 = sub->cache_tree->sha1;
> + sha1 = &sub->cache_tree->sha1;
>   mode = S_IFDIR;
>   if (sub->cache_tree->entry_count < 0)
>   to_invalidate = 1;
>   }
>   else {
> - sha1 = ce->sha1;
> + sha1 = (struct object_id *)ce->sha1;

This topic was discussed on the mailing list in the abstract.  Here is a
concrete example.

This cast is undefined, because you can't make the assumption that
cache_entry::sha1 has the same alignment and padding as (struct object_id).

I think the transition will be more tractable if you rewrite the data
structures *first*; in this case changing cache_entry::sha1 to be
(struct object_id) *before* rewriting code that works with it.

> [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html