On Wed, 13 May 2020 at 02:56, brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> 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
