Re: [PATCH] general style: replaces memcmp() with proper starts_with()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 12.03.2014 22:16, schrieb David Kastrup:
René Scharfe <l.s.r@xxxxxx> writes:

Am 12.03.2014 20:39, schrieb Junio C Hamano:
Jeff King <peff@xxxxxxxx> writes:

   static inline int standard_header_field(const char *field, size_t len)
   {
-	return ((len == 4 && !memcmp(field, "tree ", 5)) ||
-		(len == 6 && !memcmp(field, "parent ", 7)) ||
-		(len == 6 && !memcmp(field, "author ", 7)) ||
-		(len == 9 && !memcmp(field, "committer ", 10)) ||
-		(len == 8 && !memcmp(field, "encoding ", 9)));
+	return ((len == 4 && starts_with(field, "tree ")) ||
+		(len == 6 && starts_with(field, "parent ")) ||
+		(len == 6 && starts_with(field, "author ")) ||
+		(len == 9 && starts_with(field, "committer ")) ||
+		(len == 8 && starts_with(field, "encoding ")));

These extra "len" checks are interesting.  They look like an attempt to
optimize lookup, since the caller will already have scanned forward to
the space.

I wonder what the performance impact might be.  The length checks are
also needed for correctness, however, to avoid running over the end of
the buffer.

Depends on whether memcmp is guaranteed to stop immediately on mismatch.
Then memcmp(field, "tree ", 5) cannot walk across a NUL byte in field.

I'm not sure we can rely on that property. But anyway -- if field points to, say, a one-byte buffer containing the letter t, then memcmp would overrun that buffer. If field was guaranteed to be NUL-terminated then at least starts_with would stop at the end, but the signature of standard_header_field() suggests that it can be used with arbitrary buffers, not just C strings.

René
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]