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

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

[Tanay Abhra]

In record_author_date() & parse_gpg_output(), the callers of
starts_with() not just want to know if the string starts with the
prefix, but also can benefit from knowing the string that follows
the prefix.

By using skip_prefix(), we can do both at the same time.

Helped-by: Max Horn <max@quendi.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Patch V4 Identation improved, removed useless comment. [1]
		Thanks to Junio C Hamano and Max Horn.
[1] http://article.gmane.org/gmane.comp.version-control.git/243388

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 "ident_line" 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 computed as
global variables, and replacing may hamper the clarity.
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 | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..01526f7 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,14 @@ 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;
 		}
 		if (split_ident_line(&ident,
-				     buf + strlen("author "),
-				     line_end - (buf + strlen("author "))) ||
+				    		 ident_line, line_end - ident_line) ||
 		    !ident.date_begin || !ident.date_end)
 			goto fail_exit; /* malformed "author" line */
 		break;
@@ -1193,10 +1193,8 @@ 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);
+		if(!found) {
 			found = strstr(buf, sigcheck_gpg_status[i].check);
 			if (!found)
 				continue;
-- 
1.9.0

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

[Junio C Hamano]

Tanay Abhra <tanayabh@gmail.com> writes:

> In record_author_date() & parse_gpg_output(), the callers of
> starts_with() not just want to know if the string starts with the
> prefix, but also can benefit from knowing the string that follows
> the prefix.
>
> By using skip_prefix(), we can do both at the same time.
>
> Helped-by: Max Horn <max@quendi.de>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Do not add the last when sending; I never signed-off this particular
version of this patch.

> diff --git a/commit.c b/commit.c
> index 6bf4fe0..01526f7 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,14 @@ 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;
>  		}
>  		if (split_ident_line(&ident,
> -				     buf + strlen("author "),
> -				     line_end - (buf + strlen("author "))) ||
> +				    		 ident_line, line_end - ident_line) ||

Funny indentation with some SP followed by HT followed by SP.

>  		    !ident.date_begin || !ident.date_end)
>  			goto fail_exit; /* malformed "author" line */
>  		break;
> @@ -1193,10 +1193,8 @@ 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);
> +		if(!found) {
>  			found = strstr(buf, sigcheck_gpg_status[i].check);
>  			if (!found)
>  				continue;

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> +		found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
>> +		if(!found) {

Missing SP between the control keyword and parenthesized expression
the keyword uses.

I've fixed this (and the broken indentation) locally and queued the
result to 'pu', so no need to resend to correct this one.

Thanks.

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

[Tanay Abhra]

Junio C Hamano <gitster <at> pobox.com> writes:

> 
> Junio C Hamano <gitster <at> pobox.com> writes:
> 
> >> +		found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
> >> +		if(!found) {
> 
> Missing SP between the control keyword and parenthesized expression
> the keyword uses.
> 
> I've fixed this (and the broken indentation) locally and queued the
> result to 'pu', so no need to resend to correct this one.
> 
> Thanks.
> 
> 

Sorry about the indentation, it was due to not setting the tab to eight
spaces. I found your mail late, so I had already
sent a revised patch [1]. Please ignore that mail.

Also , what should be my next step ,should I present a rough draft of a
proposal , or tackle other bugs on the mailing list?

Thanks for the suggestions and advice,

Regards,
Tanay Abhra.

[1] http://article.gmane.org/gmane.comp.version-control.git/243395

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

[Junio C Hamano]

tanay abhra <tanayabh@gmail.com> writes:

> On Wed, Mar 5, 2014 at 3:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> >> +            found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
>> >> +            if(!found) {
>>
>> Missing SP between the control keyword and parenthesized expression
>> the keyword uses.
>>
>> I've fixed this (and the broken indentation) locally and queued the
>> result to 'pu', so no need to resend to correct this one.
>>
>> Thanks.
>>
>>
> Sorry about the indentation, it was due to not setting the tab to eight
> spaces. I found your mail late, so I had already
> sent a revised patch [1]. Please ignore that mail.
>
> Also , what should be my next step ,should I present a rough draft of a
> proposal , or tackle other bugs on the mailing list?

As far as I, as the maintainer of the project, am concerned [*1*],
we are done and I expect/require nothing more from you on this
change.

The MicroProject page says:

    ... If you've already done a microproject and are itching to do
    more, then get involved in other ways, like finding and fixing
    other problems in the code, or improving the documentation or
    code comments, or helping to review other people's patches on
    the mailing list, or answering questions on the mailing list or
    in IRC, or writing new tests, etc., etc. In short, start doing
    things that other Git developers do!

FYI, [GSoC 2014 timeline] (Google for it) tells us that would-be
students are supposed to be discussing project ideas with their
mentoring organizations now, in preparation for the actual student
application that begins on Mar 10 and ends on Mar 21.


[Footnote]

*1* I should mention that I am not involved in GSoC administration
and student selection for the Git project.