@matheusbsbs / matheus-bagliano / commits / 3c078b9c86

fast-import: clean up pack_data pointer in end_packfile

We have a global pointer pack_data pointing to the current pack we have open. Inside end_packfile we have two new pointers, old_p and new_p. The latter points to pack_data, and the former points to the new "installed" version of the packfile we get when we hand the file off to the regular sha1_file machinery. When then free old_p. Presumably the extra old_p pointer was there so that we could overwrite pack_data with new_p and still free old_p, but we don't do that. We just leave pack_data pointing to bogus memory, and don't overwrite it until we call start_packfile again (if ever). This can cause problems for our die routine, which calls end_packfile to clean things up. If we die at the wrong moment, we can end up looking at invalid memory in pack_data left after the last end_packfile(). Instead, let's make sure we set pack_data to NULL after we free it, and make calling endfile() again with a NULL pack_data a noop (there is nothing to end). We can further make things less confusing by dropping old_p entirely, and moving new_p closer to its point of use. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 23, 2014 at 01:27 UTC 3c078b9c860c7c1dbe8782aa1f79877354cbc602

1 file changed +7 -4

fast-import.c

+7 -4

@@ -945,10 +945,12 @@ static void unkeep_all_packs(void)

945

946 static void end_packfile(void)

947 {

948 - struct packed_git *old_p = pack_data, *new_p;

948 + if (!pack_data)

949 + return;

950

951 clear_delta_base_cache();

952 if (object_count) {

953 + struct packed_git *new_p;

954 unsigned char cur_pack_sha1[20];

955 char *idx_name;

956 int i;

@@ -990,10 +992,11 @@ static void end_packfile(void)

992 pack_id++;

993 }

994 else {

993 - close(old_p->pack_fd);

994 - unlink_or_warn(old_p->pack_name);

995 + close(pack_data->pack_fd);

996 + unlink_or_warn(pack_data->pack_name);

997 }

996 - free(old_p);

998 + free(pack_data);

999 + pack_data = NULL;

1000

1001 /* We can't carry a delta across packfiles. */

1002 strbuf_release(&last_blob.data);