upload-pack: make have_obj not global

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Oct 18, 2018 at 13:43 UTC 0b9333ff3ee172669dc3da51afa5fc0aa8e1b114
1 file changed +32 -26
upload-pack.c
+32 -26
@@ -53,7 +53,6 @@ static int no_progress, daemon_mode;
53 #define ALLOW_ANY_SHA1 07
54 static unsigned int allow_unadvertised_object_request;
55 static int shallow_nr;
56 -static struct object_array have_obj;
56 static struct object_array want_obj;
57 static struct object_array extra_edge_obj;
58 static unsigned int timeout;
@@ -100,7 +99,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
99 return 0;
100 }
101
103 -static void create_pack_file(void)
102 +static void create_pack_file(const struct object_array *have_obj)
103 {
104 struct child_process pack_objects = CHILD_PROCESS_INIT;
105 char data[8193], progress[128];
@@ -165,9 +164,9 @@ static void create_pack_file(void)
164 fprintf(pipe_fd, "%s\n",
165 oid_to_hex(&want_obj.objects[i].item->oid));
166 fprintf(pipe_fd, "--not\n");
168 - for (i = 0; i < have_obj.nr; i++)
167 + for (i = 0; i < have_obj->nr; i++)
168 fprintf(pipe_fd, "%s\n",
170 - oid_to_hex(&have_obj.objects[i].item->oid));
169 + oid_to_hex(&have_obj->objects[i].item->oid));
170 for (i = 0; i < extra_edge_obj.nr; i++)
171 fprintf(pipe_fd, "%s\n",
172 oid_to_hex(&extra_edge_obj.objects[i].item->oid));
@@ -304,7 +303,8 @@ static void create_pack_file(void)
303 die("git upload-pack: %s", abort_msg);
304 }
305
307 -static int got_oid(const char *hex, struct object_id *oid)
306 +static int got_oid(const char *hex, struct object_id *oid,
307 + struct object_array *have_obj)
308 {
309 struct object *o;
310 int we_knew_they_have = 0;
@@ -332,17 +332,17 @@ static int got_oid(const char *hex, struct object_id *oid)
332 parents->item->object.flags |= THEY_HAVE;
333 }
334 if (!we_knew_they_have) {
335 - add_object_array(o, NULL, &have_obj);
335 + add_object_array(o, NULL, have_obj);
336 return 1;
337 }
338 return 0;
339 }
340
341 -static int ok_to_give_up(void)
341 +static int ok_to_give_up(const struct object_array *have_obj)
342 {
343 uint32_t min_generation = GENERATION_NUMBER_ZERO;
344
345 - if (!have_obj.nr)
345 + if (!have_obj->nr)
346 return 0;
347
348 return can_all_from_reach_with_flag(&want_obj, THEY_HAVE,
@@ -350,7 +350,7 @@ static int ok_to_give_up(void)
350 min_generation);
351 }
352
353 -static int get_common_commits(void)
353 +static int get_common_commits(struct object_array *have_obj)
354 {
355 struct object_id oid;
356 char last_hex[GIT_MAX_HEXSZ + 1];
@@ -368,11 +368,11 @@ static int get_common_commits(void)
368
369 if (!line) {
370 if (multi_ack == 2 && got_common
371 - && !got_other && ok_to_give_up()) {
371 + && !got_other && ok_to_give_up(have_obj)) {
372 sent_ready = 1;
373 packet_write_fmt(1, "ACK %s ready\n", last_hex);
374 }
375 - if (have_obj.nr == 0 || multi_ack)
375 + if (have_obj->nr == 0 || multi_ack)
376 packet_write_fmt(1, "NAK\n");
377
378 if (no_done && sent_ready) {
@@ -386,10 +386,10 @@ static int get_common_commits(void)
386 continue;
387 }
388 if (skip_prefix(line, "have ", &arg)) {
389 - switch (got_oid(arg, &oid)) {
389 + switch (got_oid(arg, &oid, have_obj)) {
390 case -1: /* they have what we do not */
391 got_other = 1;
392 - if (multi_ack && ok_to_give_up()) {
392 + if (multi_ack && ok_to_give_up(have_obj)) {
393 const char *hex = oid_to_hex(&oid);
394 if (multi_ack == 2) {
395 sent_ready = 1;
@@ -405,14 +405,14 @@ static int get_common_commits(void)
405 packet_write_fmt(1, "ACK %s common\n", last_hex);
406 else if (multi_ack)
407 packet_write_fmt(1, "ACK %s continue\n", last_hex);
408 - else if (have_obj.nr == 1)
408 + else if (have_obj->nr == 1)
409 packet_write_fmt(1, "ACK %s\n", last_hex);
410 break;
411 }
412 continue;
413 }
414 if (!strcmp(line, "done")) {
415 - if (have_obj.nr > 0) {
415 + if (have_obj->nr > 0) {
416 if (multi_ack)
417 packet_write_fmt(1, "ACK %s\n", last_hex);
418 return 0;
@@ -1067,8 +1067,9 @@ void upload_pack(struct upload_pack_options *options)
1067
1068 receive_needs();
1069 if (want_obj.nr) {
1070 - get_common_commits();
1071 - create_pack_file();
1070 + struct object_array have_obj = OBJECT_ARRAY_INIT;
1071 + get_common_commits(&have_obj);
1072 + create_pack_file(&have_obj);
1073 }
1074 }
1075
@@ -1256,7 +1257,8 @@ static void process_args(struct packet_reader *request,
1257 }
1258 }
1259
1259 -static int process_haves(struct oid_array *haves, struct oid_array *common)
1260 +static int process_haves(struct oid_array *haves, struct oid_array *common,
1261 + struct object_array *have_obj)
1262 {
1263 int i;
1264
@@ -1289,13 +1291,14 @@ static int process_haves(struct oid_array *haves, struct oid_array *common)
1291 parents->item->object.flags |= THEY_HAVE;
1292 }
1293 if (!we_knew_they_have)
1292 - add_object_array(o, NULL, &have_obj);
1294 + add_object_array(o, NULL, have_obj);
1295 }
1296
1297 return 0;
1298 }
1299
1298 -static int send_acks(struct oid_array *acks, struct strbuf *response)
1300 +static int send_acks(struct oid_array *acks, struct strbuf *response,
1301 + const struct object_array *have_obj)
1302 {
1303 int i;
1304
@@ -1310,7 +1313,7 @@ static int send_acks(struct oid_array *acks, struct strbuf *response)
1313 oid_to_hex(&acks->oid[i]));
1314 }
1315
1313 - if (ok_to_give_up()) {
1316 + if (ok_to_give_up(have_obj)) {
1317 /* Send Ready */
1318 packet_buf_write(response, "ready\n");
1319 return 1;
@@ -1319,16 +1322,17 @@ static int send_acks(struct oid_array *acks, struct strbuf *response)
1322 return 0;
1323 }
1324
1322 -static int process_haves_and_send_acks(struct upload_pack_data *data)
1325 +static int process_haves_and_send_acks(struct upload_pack_data *data,
1326 + struct object_array *have_obj)
1327 {
1328 struct oid_array common = OID_ARRAY_INIT;
1329 struct strbuf response = STRBUF_INIT;
1330 int ret = 0;
1331
1328 - process_haves(&data->haves, &common);
1332 + process_haves(&data->haves, &common, have_obj);
1333 if (data->done) {
1334 ret = 1;
1331 - } else if (send_acks(&common, &response)) {
1335 + } else if (send_acks(&common, &response, have_obj)) {
1336 packet_buf_delim(&response);
1337 ret = 1;
1338 } else {
@@ -1394,6 +1398,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1398 {
1399 enum fetch_state state = FETCH_PROCESS_ARGS;
1400 struct upload_pack_data data;
1401 + /* NEEDSWORK: make this non-static */
1402 + static struct object_array have_obj;
1403
1404 git_config(upload_pack_config, NULL);
1405
@@ -1425,7 +1431,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1431 }
1432 break;
1433 case FETCH_SEND_ACKS:
1428 - if (process_haves_and_send_acks(&data))
1434 + if (process_haves_and_send_acks(&data, &have_obj))
1435 state = FETCH_SEND_PACK;
1436 else
1437 state = FETCH_DONE;
@@ -1435,7 +1441,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1441 send_shallow_info(&data);
1442
1443 packet_write_fmt(1, "packfile\n");
1438 - create_pack_file();
1444 + create_pack_file(&have_obj);
1445 state = FETCH_DONE;
1446 break;
1447 case FETCH_DONE: