transport: make transport vtable more private
Move the definition of the transport-specific functions provided by transports, whether declared in transport.c or transport-helper.c, into an internal header. This means that transport-using code (as opposed to transport-declaring code) can no longer access these functions (without importing the internal header themselves), making it clear that they should use the transport_*() functions instead, and also allowing the interface between the transport mechanism and an individual transport to independently evolve. This is superficially a reversal of commit 824d5776c3f2 ("Refactor struct transport_ops inlined into struct transport", 2007-09-19). However, the scope of the involved variables was neither affected nor discussed in that commit, and I think that the advantages in making those functions more private outweigh the advantages described in that commit's commit message. A minor additional point is that the code has gotten more complicated since then, in that the function-pointer variables are potentially mutated twice (once initially and once if transport_take_over() is invoked), increasing the value of corralling them into their own struct. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan committed
Dec 14, 2017 at 13:44 UTC
e967ca38473a05abf3e8f7a025c6b9bee487aa4f
4 files changed
+120
-87
transport-helper.c
+14
-9
@@ -11,6 +11,7 @@
11
#include "sigchain.h"
12
#include "argv-array.h"
13
#include "refs.h"
14
+#include "transport-internal.h"
15
16
static int debug;
17
@@ -650,7 +651,7 @@ static int fetch(struct transport *transport,
651
652
if (process_connect(transport, 0)) {
653
do_take_over(transport);
653
- return transport->fetch(transport, nr_heads, to_fetch);
654
+ return transport->vtable->fetch(transport, nr_heads, to_fetch);
655
}
656
657
count = 0;
@@ -987,7 +988,7 @@ static int push_refs(struct transport *transport,
988
989
if (process_connect(transport, 1)) {
990
do_take_over(transport);
990
- return transport->push_refs(transport, remote_refs, flags);
991
+ return transport->vtable->push_refs(transport, remote_refs, flags);
992
}
993
994
if (!remote_refs) {
@@ -1035,7 +1036,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
1036
1037
if (process_connect(transport, for_push)) {
1038
do_take_over(transport);
1038
- return transport->get_refs_list(transport, for_push);
1039
+ return transport->vtable->get_refs_list(transport, for_push);
1040
}
1041
1042
if (data->push && for_push)
@@ -1084,6 +1085,15 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
1085
return ret;
1086
}
1087
1088
+static struct transport_vtable vtable = {
1089
+ set_helper_option,
1090
+ get_refs_list,
1091
+ fetch,
1092
+ push_refs,
1093
+ connect_helper,
1094
+ release_helper
1095
+};
1096
+
1097
int transport_helper_init(struct transport *transport, const char *name)
1098
{
1099
struct helper_data *data = xcalloc(1, sizeof(*data));
@@ -1095,12 +1105,7 @@ int transport_helper_init(struct transport *transport, const char *name)
1105
debug = 1;
1106
1107
transport->data = data;
1098
- transport->set_option = set_helper_option;
1099
- transport->get_refs_list = get_refs_list;
1100
- transport->fetch = fetch;
1101
- transport->push_refs = push_refs;
1102
- transport->disconnect = release_helper;
1103
- transport->connect = connect_helper;
1108
+ transport->vtable = &vtable;
1109
transport->smart_options = &(data->transport_options);
1110
return 0;
1111
}
transport-internal.h
new
+61
@@ -0,0 +1,61 @@
1
+#ifndef TRANSPORT_INTERNAL_H
2
+#define TRANSPORT_INTERNAL_H
3
+
4
+struct ref;
5
+struct transport;
6
+
7
+struct transport_vtable {
8
+ /**
9
+ * Returns 0 if successful, positive if the option is not
10
+ * recognized or is inapplicable, and negative if the option
11
+ * is applicable but the value is invalid.
12
+ **/
13
+ int (*set_option)(struct transport *connection, const char *name,
14
+ const char *value);
15
+ /**
16
+ * Returns a list of the remote side's refs. In order to allow
17
+ * the transport to try to share connections, for_push is a
18
+ * hint as to whether the ultimate operation is a push or a fetch.
19
+ *
20
+ * If the transport is able to determine the remote hash for
21
+ * the ref without a huge amount of effort, it should store it
22
+ * in the ref's old_sha1 field; otherwise it should be all 0.
23
+ **/
24
+ struct ref *(*get_refs_list)(struct transport *transport, int for_push);
25
+
26
+ /**
27
+ * Fetch the objects for the given refs. Note that this gets
28
+ * an array, and should ignore the list structure.
29
+ *
30
+ * If the transport did not get hashes for refs in
31
+ * get_refs_list(), it should set the old_sha1 fields in the
32
+ * provided refs now.
33
+ **/
34
+ int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
35
+
36
+ /**
37
+ * Push the objects and refs. Send the necessary objects, and
38
+ * then, for any refs where peer_ref is set and
39
+ * peer_ref->new_oid is different from old_oid, tell the
40
+ * remote side to update each ref in the list from old_oid to
41
+ * peer_ref->new_oid.
42
+ *
43
+ * Where possible, set the status for each ref appropriately.
44
+ *
45
+ * The transport must modify new_sha1 in the ref to the new
46
+ * value if the remote accepted the change. Note that this
47
+ * could be a different value from peer_ref->new_oid if the
48
+ * process involved generating new commits.
49
+ **/
50
+ int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
51
+ int (*connect)(struct transport *connection, const char *name,
52
+ const char *executable, int fd[2]);
53
+
54
+ /** get_refs_list(), fetch(), and push_refs() can keep
55
+ * resources (such as a connection) reserved for further
56
+ * use. disconnect() releases these resources.
57
+ **/
58
+ int (*disconnect)(struct transport *connection);
59
+};
60
+
61
+#endif
transport.c
+43
-26
@@ -17,6 +17,7 @@
17
#include "string-list.h"
18
#include "sha1-array.h"
19
#include "sigchain.h"
20
+#include "transport-internal.h"
21
22
static void set_upstreams(struct transport *transport, struct ref *refs,
23
int pretend)
@@ -607,6 +608,15 @@ static int disconnect_git(struct transport *transport)
608
return 0;
609
}
610
611
+static struct transport_vtable taken_over_vtable = {
612
+ NULL,
613
+ get_refs_via_connect,
614
+ fetch_refs_via_pack,
615
+ git_transport_push,
616
+ NULL,
617
+ disconnect_git
618
+};
619
+
620
void transport_take_over(struct transport *transport,
621
struct child_process *child)
622
{
@@ -624,11 +634,7 @@ void transport_take_over(struct transport *transport,
634
data->got_remote_heads = 0;
635
transport->data = data;
636
627
- transport->set_option = NULL;
628
- transport->get_refs_list = get_refs_via_connect;
629
- transport->fetch = fetch_refs_via_pack;
630
- transport->push_refs = git_transport_push;
631
- transport->disconnect = disconnect_git;
637
+ transport->vtable = &taken_over_vtable;
638
transport->smart_options = &(data->options);
639
640
transport->cannot_reuse = 1;
@@ -751,6 +757,24 @@ void transport_check_allowed(const char *type)
757
die("transport '%s' not allowed", type);
758
}
759
760
+static struct transport_vtable bundle_vtable = {
761
+ NULL,
762
+ get_refs_from_bundle,
763
+ fetch_refs_from_bundle,
764
+ NULL,
765
+ NULL,
766
+ close_bundle
767
+};
768
+
769
+static struct transport_vtable builtin_smart_vtable = {
770
+ NULL,
771
+ get_refs_via_connect,
772
+ fetch_refs_via_pack,
773
+ git_transport_push,
774
+ connect_git,
775
+ disconnect_git
776
+};
777
+
778
struct transport *transport_get(struct remote *remote, const char *url)
779
{
780
const char *helper;
@@ -787,9 +811,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
811
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
812
transport_check_allowed("file");
813
ret->data = data;
790
- ret->get_refs_list = get_refs_from_bundle;
791
- ret->fetch = fetch_refs_from_bundle;
792
- ret->disconnect = close_bundle;
814
+ ret->vtable = &bundle_vtable;
815
ret->smart_options = NULL;
816
} else if (!is_url(url)
817
|| starts_with(url, "file://")
@@ -804,12 +826,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
826
*/
827
struct git_transport_data *data = xcalloc(1, sizeof(*data));
828
ret->data = data;
807
- ret->set_option = NULL;
808
- ret->get_refs_list = get_refs_via_connect;
809
- ret->fetch = fetch_refs_via_pack;
810
- ret->push_refs = git_transport_push;
811
- ret->connect = connect_git;
812
- ret->disconnect = disconnect_git;
829
+ ret->vtable = &builtin_smart_vtable;
830
ret->smart_options = &(data->options);
831
832
data->conn = NULL;
@@ -843,9 +860,9 @@ int transport_set_option(struct transport *transport,
860
git_reports = set_git_option(transport->smart_options,
861
name, value);
862
846
- if (transport->set_option)
847
- protocol_reports = transport->set_option(transport, name,
848
- value);
863
+ if (transport->vtable->set_option)
864
+ protocol_reports = transport->vtable->set_option(transport,
865
+ name, value);
866
867
/* If either report is 0, report 0 (success). */
868
if (!git_reports || !protocol_reports)
@@ -968,7 +985,7 @@ int transport_push(struct transport *transport,
985
*reject_reasons = 0;
986
transport_verify_remote_names(refspec_nr, refspec);
987
971
- if (transport->push_refs) {
988
+ if (transport->vtable->push_refs) {
989
struct ref *remote_refs;
990
struct ref *local_refs = get_local_heads();
991
int match_flags = MATCH_REFS_NONE;
@@ -981,7 +998,7 @@ int transport_push(struct transport *transport,
998
if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
999
return -1;
1000
984
- remote_refs = transport->get_refs_list(transport, 1);
1001
+ remote_refs = transport->vtable->get_refs_list(transport, 1);
1002
1003
if (flags & TRANSPORT_PUSH_ALL)
1004
match_flags |= MATCH_REFS_ALL;
@@ -1056,7 +1073,7 @@ int transport_push(struct transport *transport,
1073
}
1074
1075
if (!(flags & TRANSPORT_RECURSE_SUBMODULES_ONLY))
1059
- push_ret = transport->push_refs(transport, remote_refs, flags);
1076
+ push_ret = transport->vtable->push_refs(transport, remote_refs, flags);
1077
else
1078
push_ret = 0;
1079
err = push_had_errors(remote_refs);
@@ -1090,7 +1107,7 @@ int transport_push(struct transport *transport,
1107
const struct ref *transport_get_remote_refs(struct transport *transport)
1108
{
1109
if (!transport->got_remote_refs) {
1093
- transport->remote_refs = transport->get_refs_list(transport, 0);
1110
+ transport->remote_refs = transport->vtable->get_refs_list(transport, 0);
1111
transport->got_remote_refs = 1;
1112
}
1113
@@ -1127,7 +1144,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
1144
heads[nr_heads++] = rm;
1145
}
1146
1130
- rc = transport->fetch(transport, nr_heads, heads);
1147
+ rc = transport->vtable->fetch(transport, nr_heads, heads);
1148
1149
free(heads);
1150
return rc;
@@ -1144,8 +1161,8 @@ void transport_unlock_pack(struct transport *transport)
1161
int transport_connect(struct transport *transport, const char *name,
1162
const char *exec, int fd[2])
1163
{
1147
- if (transport->connect)
1148
- return transport->connect(transport, name, exec, fd);
1164
+ if (transport->vtable->connect)
1165
+ return transport->vtable->connect(transport, name, exec, fd);
1166
else
1167
die("Operation not supported by protocol");
1168
}
@@ -1153,8 +1170,8 @@ int transport_connect(struct transport *transport, const char *name,
1170
int transport_disconnect(struct transport *transport)
1171
{
1172
int ret = 0;
1156
- if (transport->disconnect)
1157
- ret = transport->disconnect(transport);
1173
+ if (transport->vtable->disconnect)
1174
+ ret = transport->vtable->disconnect(transport);
1175
free(transport);
1176
return ret;
1177
}
transport.h
+2
-52
@@ -30,6 +30,8 @@ enum transport_family {
30
};
31
32
struct transport {
33
+ const struct transport_vtable *vtable;
34
+
35
struct remote *remote;
36
const char *url;
37
void *data;
@@ -59,58 +61,6 @@ struct transport {
61
*/
62
const struct string_list *push_options;
63
62
- /**
63
- * Returns 0 if successful, positive if the option is not
64
- * recognized or is inapplicable, and negative if the option
65
- * is applicable but the value is invalid.
66
- **/
67
- int (*set_option)(struct transport *connection, const char *name,
68
- const char *value);
69
-
70
- /**
71
- * Returns a list of the remote side's refs. In order to allow
72
- * the transport to try to share connections, for_push is a
73
- * hint as to whether the ultimate operation is a push or a fetch.
74
- *
75
- * If the transport is able to determine the remote hash for
76
- * the ref without a huge amount of effort, it should store it
77
- * in the ref's old_sha1 field; otherwise it should be all 0.
78
- **/
79
- struct ref *(*get_refs_list)(struct transport *transport, int for_push);
80
-
81
- /**
82
- * Fetch the objects for the given refs. Note that this gets
83
- * an array, and should ignore the list structure.
84
- *
85
- * If the transport did not get hashes for refs in
86
- * get_refs_list(), it should set the old_sha1 fields in the
87
- * provided refs now.
88
- **/
89
- int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
90
-
91
- /**
92
- * Push the objects and refs. Send the necessary objects, and
93
- * then, for any refs where peer_ref is set and
94
- * peer_ref->new_oid is different from old_oid, tell the
95
- * remote side to update each ref in the list from old_oid to
96
- * peer_ref->new_oid.
97
- *
98
- * Where possible, set the status for each ref appropriately.
99
- *
100
- * The transport must modify new_sha1 in the ref to the new
101
- * value if the remote accepted the change. Note that this
102
- * could be a different value from peer_ref->new_oid if the
103
- * process involved generating new commits.
104
- **/
105
- int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
106
- int (*connect)(struct transport *connection, const char *name,
107
- const char *executable, int fd[2]);
108
-
109
- /** get_refs_list(), fetch(), and push_refs() can keep
110
- * resources (such as a connection) reserved for further
111
- * use. disconnect() releases these resources.
112
- **/
113
- int (*disconnect)(struct transport *connection);
64
char *pack_lockfile;
65
signed verbose : 3;
66
/**