Skip to content

Commit

Permalink
smart_pkt: reorder and rename parameters of git_pkt_parse_line
Browse files Browse the repository at this point in the history 
The parameters of the `git_pkt_parse_line` function are quite confusing.
First, there is no real indicator what the `out` parameter is actually
all about, and it's not really clear what the `bufflen` parameter refers
to. Reorder and rename the parameters to make this more obvious.
  • Loading branch information
@pks-t
pks-t committed Oct 3, 2018
1 parent 5fabaca commit 0b3dfbf
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 36 deletions.
2 changes: 1 addition & 1 deletion src/transports/smart.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ int git_smart__get_push_stream(transport_smart *t, git_smart_subtransport_stream
int git_smart__update_heads(transport_smart *t, git_vector *symrefs);

/* smart_pkt.c */
int git_pkt_parse_line(git_pkt **head, const char *line, const char **out, size_t len);
int git_pkt_parse_line(git_pkt **head, const char **endptr, const char *line, size_t linelen);
int git_pkt_buffer_flush(git_buf *buf);
int git_pkt_send_flush(GIT_SOCKET s);
int git_pkt_buffer_done(git_buf *buf);
Expand Down
36 changes: 18 additions & 18 deletions src/transports/smart_pkt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,13 @@ static int32_t parse_len(const char *line)
*/

int git_pkt_parse_line(
git_pkt **head, const char *line, const char **out, size_t bufflen)
git_pkt **pkt, const char **endptr, const char *line, size_t linelen)
{
int ret;
int32_t len;

/* Not even enough for the length */
if (bufflen > 0 && bufflen < PKT_LEN_SIZE)
if (linelen > 0 && linelen < PKT_LEN_SIZE)
return GIT_EBUFS;

len = parse_len(line);
Expand All @@ -422,7 +422,7 @@ int git_pkt_parse_line(
* If we fail to parse the length, it might be because the
* server is trying to send us the packfile already.
*/
if (bufflen >= 4 && !git__prefixcmp(line, "PACK")) {
if (linelen >= 4 && !git__prefixcmp(line, "PACK")) {
giterr_set(GITERR_NET, "unexpected pack file");
} else {
giterr_set(GITERR_NET, "bad packet length");
Expand All @@ -435,7 +435,7 @@ int git_pkt_parse_line(
* If we were given a buffer length, then make sure there is
* enough in the buffer to satisfy this line
*/
if (bufflen > 0 && bufflen < (size_t)len)
if (linelen > 0 && linelen < (size_t)len)
return GIT_EBUFS;

/*
Expand All @@ -458,36 +458,36 @@ int git_pkt_parse_line(
}

if (len == 0) { /* Flush pkt */
*out = line;
return flush_pkt(head);
*endptr = line;
return flush_pkt(pkt);
}

len -= PKT_LEN_SIZE; /* the encoded length includes its own size */

if (*line == GIT_SIDE_BAND_DATA)
ret = data_pkt(head, line, len);
ret = data_pkt(pkt, line, len);
else if (*line == GIT_SIDE_BAND_PROGRESS)
ret = sideband_progress_pkt(head, line, len);
ret = sideband_progress_pkt(pkt, line, len);
else if (*line == GIT_SIDE_BAND_ERROR)
ret = sideband_error_pkt(head, line, len);
ret = sideband_error_pkt(pkt, line, len);
else if (!git__prefixncmp(line, len, "ACK"))
ret = ack_pkt(head, line, len);
ret = ack_pkt(pkt, line, len);
else if (!git__prefixncmp(line, len, "NAK"))
ret = nak_pkt(head);
ret = nak_pkt(pkt);
else if (!git__prefixncmp(line, len, "ERR"))
ret = err_pkt(head, line, len);
ret = err_pkt(pkt, line, len);
else if (*line == '#')
ret = comment_pkt(head, line, len);
ret = comment_pkt(pkt, line, len);
else if (!git__prefixncmp(line, len, "ok"))
ret = ok_pkt(head, line, len);
ret = ok_pkt(pkt, line, len);
else if (!git__prefixncmp(line, len, "ng"))
ret = ng_pkt(head, line, len);
ret = ng_pkt(pkt, line, len);
else if (!git__prefixncmp(line, len, "unpack"))
ret = unpack_pkt(head, line, len);
ret = unpack_pkt(pkt, line, len);
else
ret = ref_pkt(head, line, len);
ret = ref_pkt(pkt, line, len);

*out = line + len;
*endptr = line + len;

return ret;
}
Expand Down
10 changes: 5 additions & 5 deletions src/transports/smart_protocol.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ int git_smart__store_refs(transport_smart *t, int flushes)

do {
if (buf->offset > 0)
error = git_pkt_parse_line(&pkt, buf->data, &line_end, buf->offset);
error = git_pkt_parse_line(&pkt, &line_end, buf->data, buf->offset);
else
error = GIT_EBUFS;

Expand Down Expand Up @@ -217,7 +217,7 @@ static int recv_pkt(git_pkt **out_pkt, git_pkt_type *out_type, gitno_buffer *buf

do {
if (buf->offset > 0)
error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset);
error = git_pkt_parse_line(&pkt, &line_end, ptr, buf->offset);
else
error = GIT_EBUFS;

Expand Down Expand Up @@ -755,7 +755,7 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt,
}

while (line_len > 0) {
error = git_pkt_parse_line(&pkt, line, &line_end, line_len);
error = git_pkt_parse_line(&pkt, &line_end, line, line_len);

if (error == GIT_EBUFS) {
/* Buffer the data when the inner packet is split
Expand Down Expand Up @@ -798,8 +798,8 @@ static int parse_report(transport_smart *transport, git_push *push)

for (;;) {
if (buf->offset > 0)
error = git_pkt_parse_line(&pkt, buf->data,
&line_end, buf->offset);
error = git_pkt_parse_line(&pkt, &line_end,
buf->data, buf->offset);
else
error = GIT_EBUFS;

Expand Down
24 changes: 12 additions & 12 deletions tests/transports/smart/packet.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ static void assert_flush_parses(const char *line)
const char *endptr;
git_pkt *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_FLUSH);
cl_assert_equal_strn(endptr, line + 4, linelen - 4);

Expand All @@ -25,7 +25,7 @@ static void assert_data_pkt_parses(const char *line, const char *expected_data,
const char *endptr;
git_pkt_data *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_DATA);
cl_assert_equal_i(pkt->len, expected_len);
cl_assert_equal_strn(pkt->data, expected_data, expected_len);
Expand All @@ -39,7 +39,7 @@ static void assert_sideband_progress_parses(const char *line, const char *expect
const char *endptr;
git_pkt_progress *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_PROGRESS);
cl_assert_equal_i(pkt->len, expected_len);
cl_assert_equal_strn(pkt->data, expected_data, expected_len);
Expand All @@ -53,7 +53,7 @@ static void assert_error_parses(const char *line, const char *expected_error, si
const char *endptr;
git_pkt_err *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_ERR);
cl_assert_equal_i(pkt->len, expected_len);
cl_assert_equal_strn(pkt->error, expected_error, expected_len);
Expand All @@ -70,7 +70,7 @@ static void assert_ack_parses(const char *line, const char *expected_oid, enum g

cl_git_pass(git_oid_fromstr(&oid, expected_oid));

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_ACK);
cl_assert_equal_oid(&pkt->oid, &oid);
cl_assert_equal_i(pkt->status, expected_status);
Expand All @@ -84,7 +84,7 @@ static void assert_nak_parses(const char *line)
const char *endptr;
git_pkt *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_NAK);
cl_assert_equal_strn(endptr, line + 7, linelen - 7);

Expand All @@ -97,7 +97,7 @@ static void assert_comment_parses(const char *line, const char *expected_comment
const char *endptr;
git_pkt_comment *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_COMMENT);
cl_assert_equal_strn(pkt->comment, expected_comment, strlen(expected_comment));

Expand All @@ -110,7 +110,7 @@ static void assert_ok_parses(const char *line, const char *expected_ref)
const char *endptr;
git_pkt_ok *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_OK);
cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));

Expand All @@ -123,7 +123,7 @@ static void assert_unpack_parses(const char *line, bool ok)
const char *endptr;
git_pkt_unpack *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_UNPACK);
cl_assert_equal_i(pkt->unpack_ok, ok);

Expand All @@ -136,7 +136,7 @@ static void assert_ng_parses(const char *line, const char *expected_ref, const c
const char *endptr;
git_pkt_ng *pkt;

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_NG);
cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));
cl_assert_equal_strn(pkt->msg, expected_msg, strlen(expected_msg));
Expand All @@ -156,7 +156,7 @@ static void assert_ref_parses_(const char *line, size_t linelen, const char *exp

cl_git_pass(git_oid_fromstr(&oid, expected_oid));

cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
cl_assert_equal_i(pkt->type, GIT_PKT_REF);
cl_assert_equal_oid(&pkt->head.oid, &oid);
cl_assert_equal_strn(pkt->head.name, expected_ref, strlen(expected_ref));
Expand All @@ -172,7 +172,7 @@ static void assert_pkt_fails(const char *line)
{
const char *endptr;
git_pkt *pkt;
cl_git_fail(git_pkt_parse_line(&pkt, line, &endptr, strlen(line) + 1));
cl_git_fail(git_pkt_parse_line(&pkt, &endptr, line, strlen(line) + 1));
}

void test_transports_smart_packet__parsing_garbage_fails(void)
Expand Down

0 comments on commit 0b3dfbf

Please sign in to comment.