| Message ID | 20200513005424.81369-13-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256 part 2/3: protocol functionality | expand |
On Wed, 13 May 2020 at 02:56, brian m. carlson <sandals@crustytoothpaste.net> wrote: > > We're going to be using this function in other files, so no longer mark > this function static. > > static char *server_capabilities_v1; > static struct argv_array server_capabilities_v2 = ARGV_ARRAY_INIT; > -static const char *parse_feature_value(const char *, const char *, int *, int *); > static const char *next_server_feature_value(const char *feature, int *len, int *offset); > -static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset) > +const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset) > { > --- a/connect.h > +++ b/connect.h > +const char *parse_feature_value(const char *, const char *, int *, int *); This "char *, int *" comes from the forward-declaration above, which is now dropped. Now that this is a header file for everyone to use, I think these parameters should be named, at least, but even better would be some documentation. ;-) I'll stop reading here. I'm not familiar with the technical details here (i.e., where you'd be most interested in review), so I've just left some more or less superficial comments. One thing I've noticed is that there are relatively few tests so far. I suppose it could be hard to trigger things before everything is properly plugged through. But maybe at least various error paths could be exercised already at this point, such as in the previous patch I commented on. So far I feel like I'm following along ok and I have a feeling I know where this is leading up to. Nicely done so far. Martin
diff --git a/connect.c b/connect.c index cb69aafe2c..511a069304 100644 --- a/connect.c +++ b/connect.c @@ -18,7 +18,6 @@ static char *server_capabilities_v1; static struct argv_array server_capabilities_v2 = ARGV_ARRAY_INIT; -static const char *parse_feature_value(const char *, const char *, int *, int *); static const char *next_server_feature_value(const char *feature, int *len, int *offset); static int check_ref(const char *name, unsigned int flags) @@ -467,7 +466,7 @@ struct ref **get_remote_refs(int fd_out, struct packet_reader *reader, return list; } -static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset) +const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp, int *offset) { int len; diff --git a/connect.h b/connect.h index fc75d6a457..81935a0f2a 100644 --- a/connect.h +++ b/connect.h @@ -19,6 +19,7 @@ struct packet_reader; enum protocol_version discover_version(struct packet_reader *reader); int server_supports_hash(const char *desired, int *feature_supported); +const char *parse_feature_value(const char *, const char *, int *, int *); int server_supports_v2(const char *c, int die_on_error); int server_feature_v2(const char *c, const char **v); int server_supports_feature(const char *c, const char *feature,
We're going to be using this function in other files, so no longer mark this function static. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- connect.c | 3 +-- connect.h | 1 + 2 files changed, 2 insertions(+), 2 deletions(-)