@matheusbsbs / matheus-bagliano / commits / 0380942902

pkt-line: provide a generic reading function with options

Originally we had a single function for reading packetized data: packet_read_line. Commit 46284dd grew a more "gentle" form, packet_read, that returns an error instead of dying upon reading a truncated input stream. However, it is not clear from the names which should be called, or what the difference is. Let's instead make packet_read be a generic public interface that can take option flags, and update the single callsite that uses it. This is less code, more clear, and paves the way for introducing more options into the generic interface later. The function signature is changed, so there should be no hidden conflicts with topics in flight. While we're at it, we'll document how error conditions are handled based on the options, and rename the confusing "return_line_fail" option to "gentle_on_eof". While we are cleaning up the names, we can drop the "return_line_fail" checks in packet_read_internal entirely. They look like this: ret = safe_read(..., return_line_fail); if (return_line_fail && ret < 0) ... The check for return_line_fail is a no-op; safe_read will only ever return an error value if return_line_fail was true in the first place. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 20, 2013 at 15:02 UTC 0380942902b23f02f7f595bc394e09bcd74d4ded
3 files changed +36 -15
connect.c
+2 -1
@@ -76,7 +76,8 @@ struct ref **get_remote_heads(int in, struct ref **list,
76 char *name;
77 int len, name_len;
78
79 - len = packet_read(in, buffer, sizeof(buffer));
79 + len = packet_read(in, buffer, sizeof(buffer),
80 + PACKET_READ_GENTLE_ON_EOF);
81 if (len < 0)
82 die_initial_contact(got_at_least_one_head);
83
pkt-line.c
+8 -13
@@ -103,13 +103,13 @@ void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
103 strbuf_add(buf, buffer, n);
104 }
105
106 -static int safe_read(int fd, void *buffer, unsigned size, int return_line_fail)
106 +static int safe_read(int fd, void *buffer, unsigned size, int options)
107 {
108 ssize_t ret = read_in_full(fd, buffer, size);
109 if (ret < 0)
110 die_errno("read error");
111 else if (ret < size) {
112 - if (return_line_fail)
112 + if (options & PACKET_READ_GENTLE_ON_EOF)
113 return -1;
114
115 die("The remote end hung up unexpectedly");
@@ -143,13 +143,13 @@ static int packet_length(const char *linelen)
143 return len;
144 }
145
146 -static int packet_read_internal(int fd, char *buffer, unsigned size, int return_line_fail)
146 +int packet_read(int fd, char *buffer, unsigned size, int options)
147 {
148 int len, ret;
149 char linelen[4];
150
151 - ret = safe_read(fd, linelen, 4, return_line_fail);
152 - if (return_line_fail && ret < 0)
151 + ret = safe_read(fd, linelen, 4, options);
152 + if (ret < 0)
153 return ret;
154 len = packet_length(linelen);
155 if (len < 0)
@@ -161,22 +161,17 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int return_
161 len -= 4;
162 if (len >= size)
163 die("protocol error: bad line length %d", len);
164 - ret = safe_read(fd, buffer, len, return_line_fail);
165 - if (return_line_fail && ret < 0)
164 + ret = safe_read(fd, buffer, len, options);
165 + if (ret < 0)
166 return ret;
167 buffer[len] = 0;
168 packet_trace(buffer, len, 0);
169 return len;
170 }
171
172 -int packet_read(int fd, char *buffer, unsigned size)
173 -{
174 - return packet_read_internal(fd, buffer, size, 1);
175 -}
176 -
172 int packet_read_line(int fd, char *buffer, unsigned size)
173 {
179 - return packet_read_internal(fd, buffer, size, 0);
174 + return packet_read(fd, buffer, size, 0);
175 }
176
177 int packet_get_line(struct strbuf *out,
pkt-line.h
+26 -1
@@ -24,8 +24,33 @@ void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2
24 void packet_buf_flush(struct strbuf *buf);
25 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
26
27 +/*
28 + * Read a packetized line from the descriptor into the buffer, which must be at
29 + * least size bytes long. The return value specifies the number of bytes read
30 + * into the buffer.
31 + *
32 + * If options does not contain PACKET_READ_GENTLE_ON_EOF, we will die under any
33 + * of the following conditions:
34 + *
35 + * 1. Read error from descriptor.
36 + *
37 + * 2. Protocol error from the remote (e.g., bogus length characters).
38 + *
39 + * 3. Receiving a packet larger than "size" bytes.
40 + *
41 + * 4. Truncated output from the remote (e.g., we expected a packet but got
42 + * EOF, or we got a partial packet followed by EOF).
43 + *
44 + * If options does contain PACKET_READ_GENTLE_ON_EOF, we will not die on
45 + * condition 4 (truncated input), but instead return -1. However, we will still
46 + * die for the other 3 conditions.
47 + */
48 +#define PACKET_READ_GENTLE_ON_EOF (1u<<0)
49 +int packet_read(int fd, char *buffer, unsigned size, int options);
50 +
51 +/* Historical convenience wrapper for packet_read that sets no options */
52 int packet_read_line(int fd, char *buffer, unsigned size);
28 -int packet_read(int fd, char *buffer, unsigned size);
53 +
54 int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
55
56 #endif