[PATCH v3] commit.c: use skip_prefix() instead of starts_with()

In record_author_date() & parse_gpg_output() ,using skip_prefix() instead of
starts_with() is a more suitable abstraction.

Helped-by: Max Horn [off-list ref] 
Helped-by: Junio C Hamano [off-list ref]
Helped-by: Michael Haggerty [off-list ref]
Signed-off-by: Tanay Abhra <redacted>
---
Patch V3 Variable naming improved, removed assignments inside conditionals.
	Thanks to Junio C Hamano and Max Horn.
	
Patch V2 Corrected email formatting ,reapplied the implementation according to suggestions.
	Thanks to Michael Haggerty.

This is in respect to GSoC microproject #10.

In record_author_date(), extra and useless calls to strlen due to using starts_with()
were removed by using skip_prefix(). Extra variable "skip" was used as "buf" is used in 
for loop update check.

Other usages of starts_with() in the same file can be found with,

$ grep -n starts_with commit.c

1116:		else if (starts_with(line, gpg_sig_header) &&
1196:		if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {

The starts_with() in line 1116 was left as it is, as strlen values were pre generated as 
global variables.
The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix part by
directly using the function.
Also skip_prefix() is inline when compared to starts_with().

 commit.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..6c92acb 100644
--- a/commit.c
+++ b/commit.c

@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);
 static void record_author_date(struct author_date_slab *author_date,
 			       struct commit *commit)
 {
-	const char *buf, *line_end;
+	const char *buf, *line_end, *ident_line;
 	char *buffer = NULL;
 	struct ident_split ident;
 	char *date_end;

@@ -566,14 +566,16 @@ static void record_author_date(struct author_date_slab *author_date,
 	     buf;
 	     buf = line_end + 1) {
 		line_end = strchrnul(buf, '\n');
-		if (!starts_with(buf, "author ")) {
+		ident_line = skip_prefix(buf, "author ");
+		if (!ident_line) {
 			if (!line_end[0] || line_end[1] == '\n')
 				return; /* end of header */
 			continue;
 		}
+		buf = ident_line;
 		if (split_ident_line(&ident,
-				     buf + strlen("author "),
-				     line_end - (buf + strlen("author "))) ||
+				     buf,
+				     line_end - buf) ||
 		    !ident.date_begin || !ident.date_end)
 			goto fail_exit; /* malformed "author" line */
 		break;

@@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check *sigc)
 	for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
 		const char *found, *next;
 
-		if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-			/* At the very beginning of the buffer */
-			found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-		} else {
+		found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+		/* At the very beginning of the buffer */
+		if(!found) {
 			found = strstr(buf, sigcheck_gpg_status[i].check);
 			if (!found)
 				continue;

-- 
1.9.0

On 04.03.2014, at 09:42, Tanay Abhra [off-list ref] wrote:

[...]

[...]

Why not get rid of that assignment to "buf", and use ident_line instead of buf below? That seems like it would be more readable, wouldn't it?

Do we really need that comment, and in that spot? The code seemed clear enough to me without it. But if you think keeping is better, perhaps move it to *before* the skip_prefix, and add a trailing "?"

+		if(!found) {
			found = strstr(buf, sigcheck_gpg_status[i].check);
			if (!found)
				continue;