Re: [PATCH v3 1/9] packfile: add repository to struct `packed_git`
From: Taylor Blau <hidden>
Date: 2024-10-30 20:00:09
On Wed, Oct 30, 2024 at 03:32:26PM +0100, Karthik Nayak wrote:
[...] We do need to consider that a pack file could be part of the alternates of a repository, but considering that we only have one repository struct and also that we currently anyways use 'the_repository'. We should be OK with this change.
Nicely explained.
quoted hunk ↗ jump to hunk
diff --git a/object-store-ll.h b/object-store-ll.h index 53b8e693b1..e8a22ab5fc 100644 --- a/object-store-ll.h +++ b/object-store-ll.h@@ -10,6 +10,7 @@ struct oidmap; struct oidtree; struct strbuf; +struct repository; struct object_directory { struct object_directory *next;@@ -135,6 +136,10 @@ struct packed_git { */ const uint32_t *mtimes_map; size_t mtimes_size; + + /* repo dentoes the repository this packed file belongs to */ + struct repository *r; +
Hmm. What I meant in my earlier suggestion was that we should leave the member of the struct called "repo", but change the name only in function arguments. Sorry to split hairs, but I am somewhat opposed to having such a short variable name in a struct. In either event, the comment should be made consistent with the variable name.
quoted hunk ↗ jump to hunk
/* something like ".git/objects/pack/xxxxx.pack" */ char pack_name[FLEX_ARRAY]; /* more */ };diff --git a/packfile.c b/packfile.c index 9560f0a33c..1423f23f57 100644 --- a/packfile.c +++ b/packfile.c@@ -217,11 +217,12 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value) return ntohl(level1_ofs[value]); } -static struct packed_git *alloc_packed_git(int extra) +static struct packed_git *alloc_packed_git(struct repository *r, int extra)
This spot I would leave alone.
{
struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
memset(p, 0, sizeof(*p));
p->pack_fd = -1;
+ p->r = r;
And this spot I would change to:
p->repo = r;
The rest is looking good.
Thanks,
Taylor