Re: [PATCH 1/3] pkt-line: simplify starts_with checks in packet tracing
flat view
From: Stefan Beller <hidden>
Date: 2016-06-15 23:05:17
On Fri, Jun 12, 2015 at 2:28 PM, Jeff King [off-list ref] wrote:
quoted hunk ↗ jump to hunk
We carefully check that our pkt buffer has enough characters before seeing if it starts with "PACK". The intent is to avoid reading random memory if we get a short buffer like "PAC". However, we know that the traced packets are always NUL-terminated. They come from one of these sources: 1. A string literal. 2. `format_packet`, which uses a strbuf. 3. `packet_read`, which defensively NUL-terminates what we read. We can therefore drop the length checks, as we know we will hit the trailing NUL if we have a short input. Signed-off-by: Jeff King <redacted> --- pkt-line.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)diff --git a/pkt-line.c b/pkt-line.c index 187a229..0477d2e 100644 --- a/pkt-line.c +++ b/pkt-line.c@@ -24,8 +24,7 @@ static void packet_trace(const char *buf, unsigned int len, int write) strbuf_addf(&out, "packet: %12s%c ", packet_trace_prefix, write ? '>' : '<'); - if ((len >= 4 && starts_with(buf, "PACK")) || - (len >= 5 && starts_with(buf+1, "PACK"))) {
So I wondered why there is a possible extra character in front of PACK.
So I run the blame machinery and ended up with bbc30f9963 (add
packet tracing debug code, 2011-02-24), which was also authored
by you. Where does the extra char come from?
Would it be better for readability to write it as
int offset = 0;
if (*buf == CHARACTER_STEFAN_IS_WONDERING_ABOUT)
/* ignore char foo because bar */
offset++;
if (starts_with(buf+offset, "PACK") {
...
+ if (starts_with(buf, "PACK") || starts_with(buf + 1, "PACK")) {
strbuf_addstr(&out, "PACK ...");
trace_disable(&trace_packet);
}
--
2.4.2.752.geeb594a