Re: [PATCH 17/23] midx: read objects from multi-pack-index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/9/2018 1:56 PM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 6:55 PM Derrick Stolee <stolee@xxxxxxxxx> wrote:
Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
---
  midx.c         | 96 ++++++++++++++++++++++++++++++++++++++++++++++++--
  midx.h         |  2 ++
  object-store.h |  1 +
  packfile.c     |  8 ++++-
  4 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/midx.c b/midx.c
index 5e9290ca8f..6eca8f1b12 100644
--- a/midx.c
+++ b/midx.c
@@ -3,6 +3,7 @@
  #include "dir.h"
  #include "csum-file.h"
  #include "lockfile.h"
+#include "sha1-lookup.h"
  #include "object-store.h"
  #include "packfile.h"
  #include "midx.h"
@@ -64,7 +65,7 @@ struct midxed_git *load_midxed_git(const char *object_dir)

         m = xcalloc(1, sizeof(*m) + strlen(object_dir) + 1);
         strcpy(m->object_dir, object_dir);
-       m->data = midx_map;
+       m->data = (const unsigned char*)midx_map;
Hmm? Why is this typecast only needed now? Or is it not really needed at all?

         m->signature = get_be32(m->data);
         if (m->signature != MIDX_SIGNATURE) {
@@ -145,7 +146,9 @@ struct midxed_git *load_midxed_git(const char *object_dir)

         m->num_objects = ntohl(m->chunk_oid_fanout[255]);

-       m->pack_names = xcalloc(m->num_packs, sizeof(const char *));
+       m->packs = xcalloc(m->num_packs, sizeof(*m->packs));
+
+       ALLOC_ARRAY(m->pack_names, m->num_packs);
Please make this ALLOC_ARRAY change in the patch that adds
xcalloc(m->num_packs).

         for (i = 0; i < m->num_packs; i++) {
                 if (i) {
                         if (ntohl(m->chunk_pack_lookup[i]) <= ntohl(m->chunk_pack_lookup[i - 1])) {
@@ -175,6 +178,95 @@ struct midxed_git *load_midxed_git(const char *object_dir)
         exit(1);
  }

+static int prepare_midx_pack(struct midxed_git *m, uint32_t pack_int_id)
+{
+       struct strbuf pack_name = STRBUF_INIT;
+
+       if (pack_int_id >= m->num_packs)
+               BUG("bad pack-int-id");
+
+       if (m->packs[pack_int_id])
+               return 0;
+
+       strbuf_addstr(&pack_name, m->object_dir);
+       strbuf_addstr(&pack_name, "/pack/");
+       strbuf_addstr(&pack_name, m->pack_names[pack_int_id]);
Just use strbuf_addf()

+
+       m->packs[pack_int_id] = add_packed_git(pack_name.buf, pack_name.len, 1);
+       strbuf_release(&pack_name);
+       return !m->packs[pack_int_id];
This is a weird return value convention. Normally we go zero/negative
or non-zero/zero for success/failure.

We are inconsistent.

* open_pack_index() returns non-zero on error. (This was my reference point.) * bsearch_pack() and find_pack_entry() return non-zero when an entry is found.

Since the use is "if (error) die()", similar to open_pack_index(), I'll keep the current behavior. To switch would require using "!!m->packs[pack_int_id]" here and "if (!prepare_midx_pack()) die()" in the consumer.

Thanks,
-Stolee



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux