On 6/25/2018 3:19 PM, Junio C Hamano wrote:
Derrick Stolee <sto...@gmail.com> writes:

+#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
+#define MIDX_VERSION 1
+#define MIDX_HASH_VERSION 1
+#define MIDX_HEADER_SIZE 12
+
+static char *get_midx_filename(const char *object_dir)
+{
+       return xstrfmt("%s/pack/multi-pack-index", object_dir);
+}
+
+static size_t write_midx_header(struct hashfile *f,
+                               unsigned char num_chunks,
+                               uint32_t num_packs)
+{
+       unsigned char byte_values[4];
+       hashwrite_be32(f, MIDX_SIGNATURE);
WARNING: Missing a blank line after declarations
#48: FILE: midx.c:21:
+       unsigned char byte_values[4];
+       hashwrite_be32(f, MIDX_SIGNATURE);

+       byte_values[0] = MIDX_VERSION;
+       byte_values[1] = MIDX_HASH_VERSION;
+       byte_values[2] = num_chunks;
+       byte_values[3] = 0; /* unused */
+       hashwrite(f, byte_values, sizeof(byte_values));
+       hashwrite_be32(f, num_packs);
+
+       return MIDX_HEADER_SIZE;
+}
+
  int write_midx_file(const char *object_dir)
  {
+       unsigned char num_chunks = 0;
+       char *midx_name;
+       struct hashfile *f;
+       struct lock_file lk;
+
+       midx_name = get_midx_filename(object_dir);
+       if (safe_create_leading_directories(midx_name)) {
+               UNLEAK(midx_name);
+               die_errno(_("unable to create leading directories of %s"),
+                         midx_name);
+       }
+
+       hold_lock_file_for_update(&lk, midx_name, LOCK_DIE_ON_ERROR);
+       f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);
+       FREE_AND_NULL(midx_name);
I am not sure why people prefer FREE_AND_NULL over free() for things
like this.  It is on stack; it's not like this is a static variable
visible after this function returns or anything like that.

I default to FREE_AND_NULL(X) because a later change may introduce logic to use X later in the same code block. In this case, we add a 'cleanup:' at the end which would fail in a success case if we don't set midx_name to NULL here.

I think there are some other FREE_AND_NULLs that are currently at the end of a code block, so I'll work to clean those up.


+       write_midx_header(f, num_chunks, 0);
+
+       finalize_hashfile(f, NULL, CSUM_FSYNC | CSUM_HASH_IN_STREAM);
+       commit_lock_file(&lk);
+
        return 0;
  }
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index ec3ddbe79c..8622a7cdce 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -4,7 +4,8 @@ test_description='multi-pack-indexes'
  . ./test-lib.sh
test_expect_success 'write midx with no packs' '
-       git multi-pack-index --object-dir=. write
+       git multi-pack-index --object-dir=. write &&
+       test_path_is_file pack/multi-pack-index
  '
test_done

Reply via email to