Skip to content

Commit e967ca3

Browse files
jonathantanmygitster
jonathantanmy
authored and
gitster
committed
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 824d577 ("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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 245abe3 commit e967ca3

File tree

4 files changed

+120
-87
lines changed

4 files changed

+120
-87
lines changed

‎transport-helper.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "sigchain.h"
1212
#include "argv-array.h"
1313
#include "refs.h"
14+
#include "transport-internal.h"
1415

1516
static int debug;
1617

@@ -650,7 +651,7 @@ static int fetch(struct transport *transport,
650651

651652
if (process_connect(transport, 0)) {
652653
do_take_over(transport);
653-
return transport->fetch(transport, nr_heads, to_fetch);
654+
return transport->vtable->fetch(transport, nr_heads, to_fetch);
654655
}
655656

656657
count = 0;
@@ -987,7 +988,7 @@ static int push_refs(struct transport *transport,
987988

988989
if (process_connect(transport, 1)) {
989990
do_take_over(transport);
990-
return transport->push_refs(transport, remote_refs, flags);
991+
return transport->vtable->push_refs(transport, remote_refs, flags);
991992
}
992993

993994
if (!remote_refs) {
@@ -1035,7 +1036,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
10351036

10361037
if (process_connect(transport, for_push)) {
10371038
do_take_over(transport);
1038-
return transport->get_refs_list(transport, for_push);
1039+
return transport->vtable->get_refs_list(transport, for_push);
10391040
}
10401041

10411042
if (data->push && for_push)
@@ -1084,6 +1085,15 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
10841085
return ret;
10851086
}
10861087

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+
10871097
int transport_helper_init(struct transport *transport, const char *name)
10881098
{
10891099
struct helper_data *data = xcalloc(1, sizeof(*data));
@@ -1095,12 +1105,7 @@ int transport_helper_init(struct transport *transport, const char *name)
10951105
debug = 1;
10961106

10971107
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;
11041109
transport->smart_options = &(data->transport_options);
11051110
return 0;
11061111
}

‎transport-internal.h

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "string-list.h"
1818
#include "sha1-array.h"
1919
#include "sigchain.h"
20+
#include "transport-internal.h"
2021

2122
static void set_upstreams(struct transport *transport, struct ref *refs,
2223
int pretend)
@@ -607,6 +608,15 @@ static int disconnect_git(struct transport *transport)
607608
return 0;
608609
}
609610

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+
610620
void transport_take_over(struct transport *transport,
611621
struct child_process *child)
612622
{
@@ -624,11 +634,7 @@ void transport_take_over(struct transport *transport,
624634
data->got_remote_heads = 0;
625635
transport->data = data;
626636

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;
632638
transport->smart_options = &(data->options);
633639

634640
transport->cannot_reuse = 1;
@@ -751,6 +757,24 @@ void transport_check_allowed(const char *type)
751757
die("transport '%s' not allowed", type);
752758
}
753759

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+
754778
struct transport *transport_get(struct remote *remote, const char *url)
755779
{
756780
const char *helper;
@@ -787,9 +811,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
787811
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
788812
transport_check_allowed("file");
789813
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;
793815
ret->smart_options = NULL;
794816
} else if (!is_url(url)
795817
|| starts_with(url, "file://")
@@ -804,12 +826,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
804826
*/
805827
struct git_transport_data *data = xcalloc(1, sizeof(*data));
806828
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;
813830
ret->smart_options = &(data->options);
814831

815832
data->conn = NULL;
@@ -843,9 +860,9 @@ int transport_set_option(struct transport *transport,
843860
git_reports = set_git_option(transport->smart_options,
844861
name, value);
845862

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);
849866

850867
/* If either report is 0, report 0 (success). */
851868
if (!git_reports || !protocol_reports)
@@ -968,7 +985,7 @@ int transport_push(struct transport *transport,
968985
*reject_reasons = 0;
969986
transport_verify_remote_names(refspec_nr, refspec);
970987

971-
if (transport->push_refs) {
988+
if (transport->vtable->push_refs) {
972989
struct ref *remote_refs;
973990
struct ref *local_refs = get_local_heads();
974991
int match_flags = MATCH_REFS_NONE;
@@ -981,7 +998,7 @@ int transport_push(struct transport *transport,
981998
if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
982999
return -1;
9831000

984-
remote_refs = transport->get_refs_list(transport, 1);
1001+
remote_refs = transport->vtable->get_refs_list(transport, 1);
9851002

9861003
if (flags & TRANSPORT_PUSH_ALL)
9871004
match_flags |= MATCH_REFS_ALL;
@@ -1056,7 +1073,7 @@ int transport_push(struct transport *transport,
10561073
}
10571074

10581075
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);
10601077
else
10611078
push_ret = 0;
10621079
err = push_had_errors(remote_refs);
@@ -1090,7 +1107,7 @@ int transport_push(struct transport *transport,
10901107
const struct ref *transport_get_remote_refs(struct transport *transport)
10911108
{
10921109
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);
10941111
transport->got_remote_refs = 1;
10951112
}
10961113

@@ -1127,7 +1144,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
11271144
heads[nr_heads++] = rm;
11281145
}
11291146

1130-
rc = transport->fetch(transport, nr_heads, heads);
1147+
rc = transport->vtable->fetch(transport, nr_heads, heads);
11311148

11321149
free(heads);
11331150
return rc;
@@ -1144,17 +1161,17 @@ void transport_unlock_pack(struct transport *transport)
11441161
int transport_connect(struct transport *transport, const char *name,
11451162
const char *exec, int fd[2])
11461163
{
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);
11491166
else
11501167
die("Operation not supported by protocol");
11511168
}
11521169

11531170
int transport_disconnect(struct transport *transport)
11541171
{
11551172
int ret = 0;
1156-
if (transport->disconnect)
1157-
ret = transport->disconnect(transport);
1173+
if (transport->vtable->disconnect)
1174+
ret = transport->vtable->disconnect(transport);
11581175
free(transport);
11591176
return ret;
11601177
}

‎transport.h

Lines changed: 2 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ enum transport_family {
3030
};
3131

3232
struct transport {
33+
const struct transport_vtable *vtable;
34+
3335
struct remote *remote;
3436
const char *url;
3537
void *data;
@@ -59,58 +61,6 @@ struct transport {
5961
*/
6062
const struct string_list *push_options;
6163

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);
11464
char *pack_lockfile;
11565
signed verbose : 3;
11666
/**

0 commit comments

Comments
 (0)