[PATCH v2 1/3] upload-pack: make have_obj not global

[Jonathan Tan <jonathantanmy@google.com>]

Because upload_pack_v2() can be invoked multiple times in the same
process, the static variable have_obj may not be empty when it is
invoked. To make further analysis of this situation easier, make the
variable local; analysis will be done in a subsequent patch.

The new local variable in upload_pack_v2() is static to preserve
existing behavior; this is not necessary in upload_pack() because
upload_pack() is only invoked once per process.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 upload-pack.c | 58 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index 62a1000f44..cb2401f90d 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -52,7 +52,6 @@ static int no_progress, daemon_mode;
 #define ALLOW_ANY_SHA1	07
 static unsigned int allow_unadvertised_object_request;
 static int shallow_nr;
-static struct object_array have_obj;
 static struct object_array want_obj;
 static struct object_array extra_edge_obj;
 static unsigned int timeout;
@@ -99,7 +98,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
 	return 0;
 }
 
-static void create_pack_file(void)
+static void create_pack_file(const struct object_array *have_obj)
 {
 	struct child_process pack_objects = CHILD_PROCESS_INIT;
 	char data[8193], progress[128];
@@ -164,9 +163,9 @@ static void create_pack_file(void)
 		fprintf(pipe_fd, "%s\n",
 			oid_to_hex(&want_obj.objects[i].item->oid));
 	fprintf(pipe_fd, "--not\n");
-	for (i = 0; i < have_obj.nr; i++)
+	for (i = 0; i < have_obj->nr; i++)
 		fprintf(pipe_fd, "%s\n",
-			oid_to_hex(&have_obj.objects[i].item->oid));
+			oid_to_hex(&have_obj->objects[i].item->oid));
 	for (i = 0; i < extra_edge_obj.nr; i++)
 		fprintf(pipe_fd, "%s\n",
 			oid_to_hex(&extra_edge_obj.objects[i].item->oid));
@@ -303,7 +302,8 @@ static void create_pack_file(void)
 	die("git upload-pack: %s", abort_msg);
 }
 
-static int got_oid(const char *hex, struct object_id *oid)
+static int got_oid(const char *hex, struct object_id *oid,
+		   struct object_array *have_obj)
 {
 	struct object *o;
 	int we_knew_they_have = 0;
@@ -331,17 +331,17 @@ static int got_oid(const char *hex, struct object_id *oid)
 			parents->item->object.flags |= THEY_HAVE;
 	}
 	if (!we_knew_they_have) {
-		add_object_array(o, NULL, &have_obj);
+		add_object_array(o, NULL, have_obj);
 		return 1;
 	}
 	return 0;
 }
 
-static int ok_to_give_up(void)
+static int ok_to_give_up(const struct object_array *have_obj)
 {
 	uint32_t min_generation = GENERATION_NUMBER_ZERO;
 
-	if (!have_obj.nr)
+	if (!have_obj->nr)
 		return 0;
 
 	return can_all_from_reach_with_flag(&want_obj, THEY_HAVE,
@@ -349,7 +349,7 @@ static int ok_to_give_up(void)
 					    min_generation);
 }
 
-static int get_common_commits(void)
+static int get_common_commits(struct object_array *have_obj)
 {
 	struct object_id oid;
 	char last_hex[GIT_MAX_HEXSZ + 1];
@@ -367,11 +367,11 @@ static int get_common_commits(void)
 
 		if (!line) {
 			if (multi_ack == 2 && got_common
-			    && !got_other && ok_to_give_up()) {
+			    && !got_other && ok_to_give_up(have_obj)) {
 				sent_ready = 1;
 				packet_write_fmt(1, "ACK %s ready\n", last_hex);
 			}
-			if (have_obj.nr == 0 || multi_ack)
+			if (have_obj->nr == 0 || multi_ack)
 				packet_write_fmt(1, "NAK\n");
 
 			if (no_done && sent_ready) {
@@ -385,10 +385,10 @@ static int get_common_commits(void)
 			continue;
 		}
 		if (skip_prefix(line, "have ", &arg)) {
-			switch (got_oid(arg, &oid)) {
+			switch (got_oid(arg, &oid, have_obj)) {
 			case -1: /* they have what we do not */
 				got_other = 1;
-				if (multi_ack && ok_to_give_up()) {
+				if (multi_ack && ok_to_give_up(have_obj)) {
 					const char *hex = oid_to_hex(&oid);
 					if (multi_ack == 2) {
 						sent_ready = 1;
@@ -404,14 +404,14 @@ static int get_common_commits(void)
 					packet_write_fmt(1, "ACK %s common\n", last_hex);
 				else if (multi_ack)
 					packet_write_fmt(1, "ACK %s continue\n", last_hex);
-				else if (have_obj.nr == 1)
+				else if (have_obj->nr == 1)
 					packet_write_fmt(1, "ACK %s\n", last_hex);
 				break;
 			}
 			continue;
 		}
 		if (!strcmp(line, "done")) {
-			if (have_obj.nr > 0) {
+			if (have_obj->nr > 0) {
 				if (multi_ack)
 					packet_write_fmt(1, "ACK %s\n", last_hex);
 				return 0;
@@ -1065,8 +1065,9 @@ void upload_pack(struct upload_pack_options *options)
 
 	receive_needs();
 	if (want_obj.nr) {
-		get_common_commits();
-		create_pack_file();
+		struct object_array have_obj = OBJECT_ARRAY_INIT;
+		get_common_commits(&have_obj);
+		create_pack_file(&have_obj);
 	}
 }
 
@@ -1254,7 +1255,8 @@ static void process_args(struct packet_reader *request,
 	}
 }
 
-static int process_haves(struct oid_array *haves, struct oid_array *common)
+static int process_haves(struct oid_array *haves, struct oid_array *common,
+			 struct object_array *have_obj)
 {
 	int i;
 
@@ -1287,13 +1289,14 @@ static int process_haves(struct oid_array *haves, struct oid_array *common)
 				parents->item->object.flags |= THEY_HAVE;
 		}
 		if (!we_knew_they_have)
-			add_object_array(o, NULL, &have_obj);
+			add_object_array(o, NULL, have_obj);
 	}
 
 	return 0;
 }
 
-static int send_acks(struct oid_array *acks, struct strbuf *response)
+static int send_acks(struct oid_array *acks, struct strbuf *response,
+		     const struct object_array *have_obj)
 {
 	int i;
 
@@ -1308,7 +1311,7 @@ static int send_acks(struct oid_array *acks, struct strbuf *response)
 				 oid_to_hex(&acks->oid[i]));
 	}
 
-	if (ok_to_give_up()) {
+	if (ok_to_give_up(have_obj)) {
 		/* Send Ready */
 		packet_buf_write(response, "ready\n");
 		return 1;
@@ -1317,16 +1320,17 @@ static int send_acks(struct oid_array *acks, struct strbuf *response)
 	return 0;
 }
 
-static int process_haves_and_send_acks(struct upload_pack_data *data)
+static int process_haves_and_send_acks(struct upload_pack_data *data,
+				       struct object_array *have_obj)
 {
 	struct oid_array common = OID_ARRAY_INIT;
 	struct strbuf response = STRBUF_INIT;
 	int ret = 0;
 
-	process_haves(&data->haves, &common);
+	process_haves(&data->haves, &common, have_obj);
 	if (data->done) {
 		ret = 1;
-	} else if (send_acks(&common, &response)) {
+	} else if (send_acks(&common, &response, have_obj)) {
 		packet_buf_delim(&response);
 		ret = 1;
 	} else {
@@ -1392,6 +1396,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
 {
 	enum fetch_state state = FETCH_PROCESS_ARGS;
 	struct upload_pack_data data;
+	/* NEEDSWORK: make this non-static */
+	static struct object_array have_obj;
 
 	git_config(upload_pack_config, NULL);
 
@@ -1423,7 +1429,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
 			}
 			break;
 		case FETCH_SEND_ACKS:
-			if (process_haves_and_send_acks(&data))
+			if (process_haves_and_send_acks(&data, &have_obj))
 				state = FETCH_SEND_PACK;
 			else
 				state = FETCH_DONE;
@@ -1433,7 +1439,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
 			send_shallow_info(&data);
 
 			packet_write_fmt(1, "packfile\n");
-			create_pack_file();
+			create_pack_file(&have_obj);
 			state = FETCH_DONE;
 			break;
 		case FETCH_DONE:
-- 
2.19.0.271.gfe8321ec05.dirty