From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH v2 2/3] upload-pack: make want_obj not global
Date: Thu, 18 Oct 2018 13:43:28 -0700 [thread overview]
Message-ID: <3853c006257c418ee5179aa7c35c9e9e31eeef1f.1539893192.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1539893192.git.jonathantanmy@google.com>
Because upload_pack_v2() can be invoked multiple times in the same
process, the static variable want_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 | 116 ++++++++++++++++++++++++++++----------------------
1 file changed, 66 insertions(+), 50 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index cb2401f90d..451bf47e7f 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 want_obj;
static struct object_array extra_edge_obj;
static unsigned int timeout;
static int keepalive = 5;
@@ -98,7 +97,8 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
return 0;
}
-static void create_pack_file(const struct object_array *have_obj)
+static void create_pack_file(const struct object_array *have_obj,
+ const struct object_array *want_obj)
{
struct child_process pack_objects = CHILD_PROCESS_INIT;
char data[8193], progress[128];
@@ -159,9 +159,9 @@ static void create_pack_file(const struct object_array *have_obj)
if (shallow_nr)
for_each_commit_graft(write_one_shallow, pipe_fd);
- for (i = 0; i < want_obj.nr; i++)
+ for (i = 0; i < want_obj->nr; i++)
fprintf(pipe_fd, "%s\n",
- oid_to_hex(&want_obj.objects[i].item->oid));
+ oid_to_hex(&want_obj->objects[i].item->oid));
fprintf(pipe_fd, "--not\n");
for (i = 0; i < have_obj->nr; i++)
fprintf(pipe_fd, "%s\n",
@@ -337,19 +337,21 @@ static int got_oid(const char *hex, struct object_id *oid,
return 0;
}
-static int ok_to_give_up(const struct object_array *have_obj)
+static int ok_to_give_up(const struct object_array *have_obj,
+ struct object_array *want_obj)
{
uint32_t min_generation = GENERATION_NUMBER_ZERO;
if (!have_obj->nr)
return 0;
- return can_all_from_reach_with_flag(&want_obj, THEY_HAVE,
+ return can_all_from_reach_with_flag(want_obj, THEY_HAVE,
COMMON_KNOWN, oldest_have,
min_generation);
}
-static int get_common_commits(struct object_array *have_obj)
+static int get_common_commits(struct object_array *have_obj,
+ struct object_array *want_obj)
{
struct object_id oid;
char last_hex[GIT_MAX_HEXSZ + 1];
@@ -367,7 +369,7 @@ static int get_common_commits(struct object_array *have_obj)
if (!line) {
if (multi_ack == 2 && got_common
- && !got_other && ok_to_give_up(have_obj)) {
+ && !got_other && ok_to_give_up(have_obj, want_obj)) {
sent_ready = 1;
packet_write_fmt(1, "ACK %s ready\n", last_hex);
}
@@ -388,7 +390,7 @@ static int get_common_commits(struct object_array *have_obj)
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(have_obj)) {
+ if (multi_ack && ok_to_give_up(have_obj, want_obj)) {
const char *hex = oid_to_hex(&oid);
if (multi_ack == 2) {
sent_ready = 1;
@@ -581,7 +583,7 @@ static int has_unreachable(struct object_array *src)
return 1;
}
-static void check_non_tip(void)
+static void check_non_tip(struct object_array *want_obj)
{
int i;
@@ -592,14 +594,14 @@ static void check_non_tip(void)
*/
if (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))
goto error;
- if (!has_unreachable(&want_obj))
+ if (!has_unreachable(want_obj))
/* All the non-tip ones are ancestors of what we advertised */
return;
error:
/* Pick one of them (we know there at least is one) */
- for (i = 0; i < want_obj.nr; i++) {
- struct object *o = want_obj.objects[i].item;
+ for (i = 0; i < want_obj->nr; i++) {
+ struct object *o = want_obj->objects[i].item;
if (!is_our_ref(o))
die("git upload-pack: not our ref %s",
oid_to_hex(&o->oid));
@@ -620,7 +622,8 @@ static void send_shallow(struct commit_list *result)
}
}
-static void send_unshallow(const struct object_array *shallows)
+static void send_unshallow(const struct object_array *shallows,
+ struct object_array *want_obj)
{
int i;
@@ -644,7 +647,7 @@ static void send_unshallow(const struct object_array *shallows)
parents = ((struct commit *)object)->parents;
while (parents) {
add_object_array(&parents->item->object,
- NULL, &want_obj);
+ NULL, want_obj);
parents = parents->next;
}
add_object_array(object, NULL, &extra_edge_obj);
@@ -655,7 +658,7 @@ static void send_unshallow(const struct object_array *shallows)
}
static void deepen(int depth, int deepen_relative,
- struct object_array *shallows)
+ struct object_array *shallows, struct object_array *want_obj)
{
if (depth == INFINITE_DEPTH && !is_repository_shallow(the_repository)) {
int i;
@@ -678,38 +681,40 @@ static void deepen(int depth, int deepen_relative,
} else {
struct commit_list *result;
- result = get_shallow_commits(&want_obj, depth,
+ result = get_shallow_commits(want_obj, depth,
SHALLOW, NOT_SHALLOW);
send_shallow(result);
free_commit_list(result);
}
- send_unshallow(shallows);
+ send_unshallow(shallows, want_obj);
}
static void deepen_by_rev_list(int ac, const char **av,
- struct object_array *shallows)
+ struct object_array *shallows,
+ struct object_array *want_obj)
{
struct commit_list *result;
result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW);
send_shallow(result);
free_commit_list(result);
- send_unshallow(shallows);
+ send_unshallow(shallows, want_obj);
}
/* Returns 1 if a shallow list is sent or 0 otherwise */
static int send_shallow_list(int depth, int deepen_rev_list,
timestamp_t deepen_since,
struct string_list *deepen_not,
- struct object_array *shallows)
+ struct object_array *shallows,
+ struct object_array *want_obj)
{
int ret = 0;
if (depth > 0 && deepen_rev_list)
die("git upload-pack: deepen and deepen-since (or deepen-not) cannot be used together");
if (depth > 0) {
- deepen(depth, deepen_relative, shallows);
+ deepen(depth, deepen_relative, shallows, want_obj);
ret = 1;
} else if (deepen_rev_list) {
struct argv_array av = ARGV_ARRAY_INIT;
@@ -726,11 +731,11 @@ static int send_shallow_list(int depth, int deepen_rev_list,
}
argv_array_push(&av, "--not");
}
- for (i = 0; i < want_obj.nr; i++) {
- struct object *o = want_obj.objects[i].item;
+ for (i = 0; i < want_obj->nr; i++) {
+ struct object *o = want_obj->objects[i].item;
argv_array_push(&av, oid_to_hex(&o->oid));
}
- deepen_by_rev_list(av.argc, av.argv, shallows);
+ deepen_by_rev_list(av.argc, av.argv, shallows, want_obj);
argv_array_clear(&av);
ret = 1;
} else {
@@ -815,7 +820,7 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
return 0;
}
-static void receive_needs(void)
+static void receive_needs(struct object_array *want_obj)
{
struct object_array shallows = OBJECT_ARRAY_INIT;
struct string_list deepen_not = STRING_LIST_INIT_DUP;
@@ -893,7 +898,7 @@ static void receive_needs(void)
if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
|| is_our_ref(o)))
has_non_tip = 1;
- add_object_array(o, NULL, &want_obj);
+ add_object_array(o, NULL, want_obj);
}
}
@@ -905,7 +910,7 @@ static void receive_needs(void)
* by another process that handled the initial request.
*/
if (has_non_tip)
- check_non_tip();
+ check_non_tip(want_obj);
if (!use_sideband && daemon_mode)
no_progress = 1;
@@ -914,7 +919,7 @@ static void receive_needs(void)
return;
if (send_shallow_list(depth, deepen_rev_list, deepen_since,
- &deepen_not, &shallows))
+ &deepen_not, &shallows, want_obj))
packet_flush(1);
object_array_clear(&shallows);
}
@@ -1040,6 +1045,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
void upload_pack(struct upload_pack_options *options)
{
struct string_list symref = STRING_LIST_INIT_DUP;
+ struct object_array want_obj = OBJECT_ARRAY_INIT;
stateless_rpc = options->stateless_rpc;
timeout = options->timeout;
@@ -1063,11 +1069,11 @@ void upload_pack(struct upload_pack_options *options)
if (options->advertise_refs)
return;
- receive_needs();
+ receive_needs(&want_obj);
if (want_obj.nr) {
struct object_array have_obj = OBJECT_ARRAY_INIT;
- get_common_commits(&have_obj);
- create_pack_file(&have_obj);
+ get_common_commits(&have_obj, &want_obj);
+ create_pack_file(&have_obj, &want_obj);
}
}
@@ -1117,7 +1123,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
string_list_clear(&data->deepen_not, 0);
}
-static int parse_want(const char *line)
+static int parse_want(const char *line, struct object_array *want_obj)
{
const char *arg;
if (skip_prefix(line, "want ", &arg)) {
@@ -1139,7 +1145,7 @@ static int parse_want(const char *line)
if (!(o->flags & WANTED)) {
o->flags |= WANTED;
- add_object_array(o, NULL, &want_obj);
+ add_object_array(o, NULL, want_obj);
}
return 1;
@@ -1148,7 +1154,8 @@ static int parse_want(const char *line)
return 0;
}
-static int parse_want_ref(const char *line, struct string_list *wanted_refs)
+static int parse_want_ref(const char *line, struct string_list *wanted_refs,
+ struct object_array *want_obj)
{
const char *arg;
if (skip_prefix(line, "want-ref ", &arg)) {
@@ -1167,7 +1174,7 @@ static int parse_want_ref(const char *line, struct string_list *wanted_refs)
o = parse_object_or_die(&oid, arg);
if (!(o->flags & WANTED)) {
o->flags |= WANTED;
- add_object_array(o, NULL, &want_obj);
+ add_object_array(o, NULL, want_obj);
}
return 1;
@@ -1192,16 +1199,18 @@ static int parse_have(const char *line, struct oid_array *haves)
}
static void process_args(struct packet_reader *request,
- struct upload_pack_data *data)
+ struct upload_pack_data *data,
+ struct object_array *want_obj)
{
while (packet_reader_read(request) != PACKET_READ_FLUSH) {
const char *arg = request->line;
const char *p;
/* process want */
- if (parse_want(arg))
+ if (parse_want(arg, want_obj))
continue;
- if (allow_ref_in_want && parse_want_ref(arg, &data->wanted_refs))
+ if (allow_ref_in_want &&
+ parse_want_ref(arg, &data->wanted_refs, want_obj))
continue;
/* process have line */
if (parse_have(arg, &data->haves))
@@ -1296,7 +1305,8 @@ static int process_haves(struct oid_array *haves, struct oid_array *common,
}
static int send_acks(struct oid_array *acks, struct strbuf *response,
- const struct object_array *have_obj)
+ const struct object_array *have_obj,
+ struct object_array *want_obj)
{
int i;
@@ -1311,7 +1321,7 @@ static int send_acks(struct oid_array *acks, struct strbuf *response,
oid_to_hex(&acks->oid[i]));
}
- if (ok_to_give_up(have_obj)) {
+ if (ok_to_give_up(have_obj, want_obj)) {
/* Send Ready */
packet_buf_write(response, "ready\n");
return 1;
@@ -1321,7 +1331,8 @@ static int send_acks(struct oid_array *acks, struct strbuf *response,
}
static int process_haves_and_send_acks(struct upload_pack_data *data,
- struct object_array *have_obj)
+ struct object_array *have_obj,
+ struct object_array *want_obj)
{
struct oid_array common = OID_ARRAY_INIT;
struct strbuf response = STRBUF_INIT;
@@ -1330,7 +1341,7 @@ static int process_haves_and_send_acks(struct upload_pack_data *data,
process_haves(&data->haves, &common, have_obj);
if (data->done) {
ret = 1;
- } else if (send_acks(&common, &response, have_obj)) {
+ } else if (send_acks(&common, &response, have_obj, want_obj)) {
packet_buf_delim(&response);
ret = 1;
} else {
@@ -1366,7 +1377,8 @@ static void send_wanted_ref_info(struct upload_pack_data *data)
packet_delim(1);
}
-static void send_shallow_info(struct upload_pack_data *data)
+static void send_shallow_info(struct upload_pack_data *data,
+ struct object_array *want_obj)
{
/* No shallow info needs to be sent */
if (!data->depth && !data->deepen_rev_list && !data->shallows.nr &&
@@ -1377,9 +1389,10 @@ static void send_shallow_info(struct upload_pack_data *data)
if (!send_shallow_list(data->depth, data->deepen_rev_list,
data->deepen_since, &data->deepen_not,
- &data->shallows) &&
+ &data->shallows, want_obj) &&
is_repository_shallow(the_repository))
- deepen(INFINITE_DEPTH, data->deepen_relative, &data->shallows);
+ deepen(INFINITE_DEPTH, data->deepen_relative, &data->shallows,
+ want_obj);
packet_delim(1);
}
@@ -1398,6 +1411,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
struct upload_pack_data data;
/* NEEDSWORK: make this non-static */
static struct object_array have_obj;
+ /* NEEDSWORK: make this non-static */
+ static struct object_array want_obj;
git_config(upload_pack_config, NULL);
@@ -1407,7 +1422,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
while (state != FETCH_DONE) {
switch (state) {
case FETCH_PROCESS_ARGS:
- process_args(request, &data);
+ process_args(request, &data, &want_obj);
if (!want_obj.nr) {
/*
@@ -1429,17 +1444,18 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
}
break;
case FETCH_SEND_ACKS:
- if (process_haves_and_send_acks(&data, &have_obj))
+ if (process_haves_and_send_acks(&data, &have_obj,
+ &want_obj))
state = FETCH_SEND_PACK;
else
state = FETCH_DONE;
break;
case FETCH_SEND_PACK:
send_wanted_ref_info(&data);
- send_shallow_info(&data);
+ send_shallow_info(&data, &want_obj);
packet_write_fmt(1, "packfile\n");
- create_pack_file(&have_obj);
+ create_pack_file(&have_obj, &want_obj);
state = FETCH_DONE;
break;
case FETCH_DONE:
--
2.19.0.271.gfe8321ec05.dirty
next prev parent reply other threads:[~2018-10-18 20:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-16 21:58 [PATCH] upload-pack: clear flags before each v2 request Jonathan Tan
2018-10-17 12:05 ` Arturas Moskvinas
2018-10-18 5:16 ` Junio C Hamano
2018-10-18 20:43 ` [PATCH v2 0/3] Clear " Jonathan Tan
2018-10-18 20:43 ` [PATCH v2 1/3] upload-pack: make have_obj not global Jonathan Tan
2018-10-18 20:43 ` Jonathan Tan [this message]
2018-10-18 20:43 ` [PATCH v2 3/3] upload-pack: clear flags before each v2 request Jonathan Tan
2018-10-19 3:19 ` [PATCH v2 0/3] Clear " Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3853c006257c418ee5179aa7c35c9e9e31eeef1f.1539893192.git.jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.