threads / patch / 36215

v5use starts_with() instead of !memcmp()

Subject: [PATCH v5] use starts_with() instead of !memcmp()

## tl;dr

5 messages between Mar 19, 2014 and Mar 21, 2014. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Quint Guvernator· Mar 19, 2014, 01:18 UTC · lore

Another version, this time very in line with the review and commentary of Junio, Eric, and Michael. This version boasts a revamped commit message and fewer but surer hunks changed.

Thanks again for the guidance.
Quint Guvernator (1):
  use starts_with() instead of !memcmp()
 builtin/apply.c        |  4 ++--
 builtin/for-each-ref.c |  2 +-
 builtin/mktag.c        |  2 +-
 builtin/patch-id.c     | 10 +++++-----
 connect.c              |  4 ++--
 imap-send.c            |  6 +++---
 remote.c               |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)
-- 
1.9.0
Quint Guvernator· Mar 19, 2014, 01:18 UTC · re: Quint Guvernator · lore

When checking if a string begins with a constant string, using starts_with() indicates the intention of the check more clearly and is less error-prone than calling !memcmp() with an explicit byte count.

Signed-off-by: Quint Guvernator <quintus.public@gmail.com>
---
 builtin/apply.c        |  4 ++--
 builtin/for-each-ref.c |  2 +-
 builtin/mktag.c        |  2 +-
 builtin/patch-id.c     | 10 +++++-----
 connect.c              |  4 ++--
 imap-send.c            |  6 +++---
 remote.c               |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)
Show changes to 7 files +15 −15

builtin/apply.c, builtin/for-each-ref.c, builtin/mktag.c, builtin/patch-id.c, connect.c, imap-send.c, remote.c

diff --git a/builtin/apply.c b/builtin/apply.c
index 0189523..826b3e2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1631,7 +1631,7 @@ static int parse_fragment(const char *line, unsigned long size,
 		 * l10n of "\ No newline..." is at least that long.
 		 */
 		case '\\':
-			if (len < 12 || memcmp(line, "\\ ", 2))
+			if (len < 12 || !starts_with(line, "\\ "))
 				return -1;
 			break;
 		}
@@ -1646,7 +1646,7 @@ static int parse_fragment(const char *line, unsigned long size,
 	 * it in the above loop because we hit oldlines == newlines == 0
 	 * before seeing it.
 	 */
-	if (12 < size && !memcmp(line, "\\ ", 2))
+	if (12 < size && starts_with(line, "\\ "))
 		offset += linelen(line, size);
 
 	patch->lines_added += added;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3e1d5c3..4135980 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -193,7 +193,7 @@ static int verify_format(const char *format)
 		at = parse_atom(sp + 2, ep);
 		cp = ep + 1;
 
-		if (!memcmp(used_atom[at], "color:", 6))
+		if (starts_with(used_atom[at], "color:"))
 			need_color_reset_at_eol = !!strcmp(used_atom[at], color_reset);
 	}
 	return 0;
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 640ab64..d2d9310 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -57,7 +57,7 @@ static int verify_tag(char *buffer, unsigned long size)
 
 	/* Verify type line */
 	type_line = object + 48;
-	if (memcmp(type_line - 1, "\ntype ", 6))
+	if (!starts_with(type_line - 1, "\ntype "))
 		return error("char%d: could not find \"\\ntype \"", 47);
 
 	/* Verify tag-line */
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..23ef424 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -81,14 +81,14 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st
 		}
 
 		/* Ignore commit comments */
-		if (!patchlen && memcmp(line, "diff ", 5))
+		if (!patchlen && !starts_with(line, "diff "))
 			continue;
 
 		/* Parsing diff header?  */
 		if (before == -1) {
-			if (!memcmp(line, "index ", 6))
+			if (starts_with(line, "index "))
 				continue;
-			else if (!memcmp(line, "--- ", 4))
+			else if (starts_with(line, "--- "))
 				before = after = 1;
 			else if (!isalpha(line[0]))
 				break;
@@ -96,14 +96,14 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st
 
 		/* Looking for a valid hunk header?  */
 		if (before == 0 && after == 0) {
-			if (!memcmp(line, "@@ -", 4)) {
+			if (starts_with(line, "@@ -")) {
 				/* Parse next hunk, but ignore line numbers.  */
 				scan_hunk_header(line, &before, &after);
 				continue;
 			}
 
 			/* Split at the end of the patch.  */
-			if (memcmp(line, "diff ", 5))
+			if (!starts_with(line, "diff "))
 				break;
 
 			/* Else we're parsing another header.  */
diff --git a/connect.c b/connect.c
index 4150412..5ae2aaa 100644
--- a/connect.c
+++ b/connect.c
@@ -30,11 +30,11 @@ static int check_ref(const char *name, int len, unsigned int flags)
 		return 0;
 
 	/* REF_HEADS means that we want regular branch heads */
-	if ((flags & REF_HEADS) && !memcmp(name, "heads/", 6))
+	if ((flags & REF_HEADS) && starts_with(name, "heads/"))
 		return 1;
 
 	/* REF_TAGS means that we want tags */
-	if ((flags & REF_TAGS) && !memcmp(name, "tags/", 5))
+	if ((flags & REF_TAGS) && starts_with(name, "tags/"))
 		return 1;
 
 	/* All type bits clear means that we are ok with anything */
diff --git a/imap-send.c b/imap-send.c
index 0bc6f7f..019de18 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -524,7 +524,7 @@ static struct imap_cmd *v_issue_imap_cmd(struct imap_store *ctx,
 	if (Verbose) {
 		if (imap->num_in_progress)
 			printf("(%d in progress) ", imap->num_in_progress);
-		if (memcmp(cmd->cmd, "LOGIN", 5))
+		if (!starts_with(cmd->cmd, "LOGIN"))
 			printf(">>> %s", buf);
 		else
 			printf(">>> %d LOGIN <user> <pass>\n", cmd->tag);
@@ -802,7 +802,7 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
 				resp = DRV_OK;
 			else {
 				if (!strcmp("NO", arg)) {
-					if (cmdp->cb.create && cmd && (cmdp->cb.trycreate || !memcmp(cmd, "[TRYCREATE]", 11))) { /* SELECT, APPEND or UID COPY */
+					if (cmdp->cb.create && cmd && (cmdp->cb.trycreate || starts_with(cmd, "[TRYCREATE]"))) { /* SELECT, APPEND or UID COPY */
 						p = strchr(cmdp->cmd, '"');
 						if (!issue_imap_cmd(ctx, NULL, "CREATE \"%.*s\"", (int)(strchr(p + 1, '"') - p + 1), p)) {
 							resp = RESP_BAD;
@@ -827,7 +827,7 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
 				} else /*if (!strcmp("BAD", arg))*/
 					resp = RESP_BAD;
 				fprintf(stderr, "IMAP command '%s' returned response (%s) - %s\n",
-					 memcmp(cmdp->cmd, "LOGIN", 5) ?
+					 !starts_with(cmdp->cmd, "LOGIN") ?
 							cmdp->cmd : "LOGIN <user> <pass>",
 							arg, cmd ? cmd : "");
 			}
diff --git a/remote.c b/remote.c
index 5f63d55..bd02b0e 100644
--- a/remote.c
+++ b/remote.c
@@ -1149,7 +1149,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
+		if (starts_with(dst_value, "refs/"))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else if (is_null_sha1(matched_src->new_sha1))
 			error("unable to delete '%s': remote ref does not exist",
-- 
1.9.0
Eric Sunshine· Mar 19, 2014, 04:13 UTC · re: Quint Guvernator · lore

Re: [PATCH v5] use starts_with() instead of !memcmp()

On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator <quintus.public@gmail.com> wrote:

> Another version, this time very in line with the review and commentary of
> Junio, Eric, and Michael.  This version boasts a revamped commit message and
> fewer but surer hunks changed.

Explaining what changed in this version is indeed a courtesy to reviewers. Thanks.

For bonus points, provide a link to the previous attempt, like this [1].
[1]: http://thread.gmane.org/gmane.comp.version-control.git/244292
Show 16 quoted lines
> Thanks again for the guidance.
>
> Quint Guvernator (1):
>   use starts_with() instead of !memcmp()
>
>  builtin/apply.c        |  4 ++--
>  builtin/for-each-ref.c |  2 +-
>  builtin/mktag.c        |  2 +-
>  builtin/patch-id.c     | 10 +++++-----
>  connect.c              |  4 ++--
>  imap-send.c            |  6 +++---
>  remote.c               |  2 +-
>  7 files changed, 15 insertions(+), 15 deletions(-)
>
> --
> 1.9.0
Junio C Hamano· Mar 21, 2014, 16:53 UTC · re: Eric Sunshine · lore

Re: [PATCH v5] use starts_with() instead of !memcmp()

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 8 quoted lines
> On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator
> <quintus.public@gmail.com> wrote:
>> Another version, this time very in line with the review and commentary of
>> Junio, Eric, and Michael.  This version boasts a revamped commit message and
>> fewer but surer hunks changed.
>
> Explaining what changed in this version is indeed a courtesy to
> reviewers. Thanks.
So, is that a "reviewed-by: Eric"?
Eric Sunshine· Mar 21, 2014, 18:05 UTC · re: Junio C Hamano · lore

Re: [PATCH v5] use starts_with() instead of !memcmp()

On Fri, Mar 21, 2014 at 12:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Tue, Mar 18, 2014 at 9:18 PM, Quint Guvernator
>> <quintus.public@gmail.com> wrote:
>>> Another version, this time very in line with the review and commentary of
>>> Junio, Eric, and Michael.  This version boasts a revamped commit message and
>>> fewer but surer hunks changed.
>>
>> Explaining what changed in this version is indeed a courtesy to
>> reviewers. Thanks.
>
> So, is that a "reviewed-by: Eric"?

No, sorry. You and Peff were actively reviewing Quint's submissions, so I merely scanned them quickly without making a careful examination. I've been commenting upon so many GSoC submissions that it's hard to remember which is which, and upon reading "Another version, this time very in line with the review and commentary Junio, Eric, and Michael", I had to search the list archive to figure out why my name was listed. Hence, the suggestion from me that providing a link to the previous attempt is a welcome courtesy.

← back to recent threads