[PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
In record_author_date() : Replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable indent_line. In parse_signed_commit() : Replace line + gpg_sig_header_len by skip_prefix(), which is saved in a new const char variable indent_line. In parse_gpg_output() : Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by skip_prefix(), which is saved in already available variable found. Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c @@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab *author_date, struct ident_split ident; char *date_end; unsigned long date; + const char *indent_line; if (!commit-buffer) { unsigned long size; @@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab *author_date, return; } + indent_line = skip_prefix(buf, author ); + for (buf = commit-buffer ? commit-buffer : buffer; buf; buf = line_end + 1) { line_end = strchrnul(buf, '\n'); - if (!starts_with(buf, author )) { + if (!indent_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 ))) || + if (split_ident_line(ident, indent_line, +line_end - indent_line) || !ident.date_begin || !ident.date_end) goto fail_exit; /* malformed author line */ break; @@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1, char *buffer = read_sha1_file(sha1, type, size); int in_signature, saw_signature = -1; char *line, *tail; + const char *indent_line; if (!buffer || type != OBJ_COMMIT) goto cleanup; @@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) -line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') + sig = indent_line + 2; if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; @@ -1193,10 +1196,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.138.g2de3478 Hey Eric, As per your suggestion in the previous mail : http://article.gmane.org/gmane.comp.version-control.git/243316 I have made necessary changes: 1. Better Naming of variables as per suggestion 2. Proper replacement of skip_prefix() over starts_with() . -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Karthik Nayak karthik.188 at gmail.com writes: In record_author_date() : Replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable indent_line. In parse_signed_commit() : Replace line + gpg_sig_header_len by skip_prefix(), which is saved in a new const char variable indent_line. In parse_gpg_output() : Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by skip_prefix(), which is saved in already available variable found. Signed-off-by: Karthik Nayak karthik.188 at gmail.com --- commit.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c at at -1098,6 +1100,7 at at int parse_signed_commit(const unsigned char *sha1, char *buffer = read_sha1_file(sha1, type, size); int in_signature, saw_signature = -1; char *line, *tail; + const char *indent_line; if (!buffer || type != OBJ_COMMIT) goto cleanup; at at -,11 +1114,11 at at int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) - line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') + sig = indent_line + 2; if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; Hi, I was attempting the same microproject so I thought I would share some points that were told to me earlier .The mail subject should have a increasing order of submission numbers for each revision(for example here it should be [PATCH v2]). Also write the superfluous stuff which you have written in the bottom after ---(the three dashes after the signed off statement) . In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header variables are precomputed outside of the function, and also Junio said to prefer starts_with(), whenever skip_prefix() advantages are not so important in the context.So the replace may not be so advantageous ,I may be wrong in this case. Cheers, Tanay Abhra. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Hey Tanay, 1. Yes just getting used to git send-email now, should follow that from now 2. I thought it shouldn't be a part of the commit, so i put it after the last --- 3. I did have a thought on your lines also , but concluded to it being more advantageous, you might be right though Nice to hear from you. Thanks -Karthik Nayak On Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra tanay...@gmail.com wrote: Karthik Nayak karthik.188 at gmail.com writes: In record_author_date() : Replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable indent_line. In parse_signed_commit() : Replace line + gpg_sig_header_len by skip_prefix(), which is saved in a new const char variable indent_line. In parse_gpg_output() : Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by skip_prefix(), which is saved in already available variable found. Signed-off-by: Karthik Nayak karthik.188 at gmail.com --- commit.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c at at -1098,6 +1100,7 at at int parse_signed_commit(const unsigned char *sha1, char *buffer = read_sha1_file(sha1, type, size); int in_signature, saw_signature = -1; char *line, *tail; + const char *indent_line; if (!buffer || type != OBJ_COMMIT) goto cleanup; at at -,11 +1114,11 at at int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) - line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') + sig = indent_line + 2; if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; Hi, I was attempting the same microproject so I thought I would share some points that were told to me earlier .The mail subject should have a increasing order of submission numbers for each revision(for example here it should be [PATCH v2]). Also write the superfluous stuff which you have written in the bottom after ---(the three dashes after the signed off statement) . In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header variables are precomputed outside of the function, and also Junio said to prefer starts_with(), whenever skip_prefix() advantages are not so important in the context.So the replace may not be so advantageous ,I may be wrong in this case. Cheers, Tanay Abhra. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
Thanks for the resend. Etiquette on this list is to cc: people who commented on previous versions of the submission. As Tanay already mentioned, use [PATCH vN] in the subject where N is the version number of this attempt. The -v option of git format-email can help. More below. On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote: In record_author_date() : Replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable indent_line. In parse_signed_commit() : Replace line + gpg_sig_header_len by skip_prefix(), which is saved in a new const char variable indent_line. In parse_gpg_output() : Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by skip_prefix(), which is saved in already available variable found. It's not necessary to explain in prose what the patch itself already states more concisely and precisely. All of this text should be dropped from the commit message. Instead, explain the purpose of the patch, the problem it solves, etc. Please read the (2) Describe your changes well section of Documentation/SubmittingPatches to get an idea of what sort of information to include in the commit message. A better commit message should say something about how the affected functions want to know not only if the string has a prefix, but also the text following the prefix, and that skip_prefix() can provide both pieces of information. Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c @@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab *author_date, struct ident_split ident; char *date_end; unsigned long date; + const char *indent_line; Strange variable name. The code in question splits apart a person's identification string (name, email, etc.). It has nothing to do with indentation. if (!commit-buffer) { unsigned long size; @@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab *author_date, return; } + indent_line = skip_prefix(buf, author ); + for (buf = commit-buffer ? commit-buffer : buffer; buf; buf = line_end + 1) { line_end = strchrnul(buf, '\n'); - if (!starts_with(buf, author )) { + if (!indent_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 ))) || + if (split_ident_line(ident, indent_line, +line_end - indent_line) || Indent the second line (using tabs plus possible spaces) so that it lines up with the ident in the line above. Be sure to set your editor so tabs have width 8. !ident.date_begin || !ident.date_end) goto fail_exit; /* malformed author line */ break; @@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1, char *buffer = read_sha1_file(sha1, type, size); int in_signature, saw_signature = -1; char *line, *tail; + const char *indent_line; if (!buffer || type != OBJ_COMMIT) goto cleanup; @@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); Even stranger variable name for a GPG signature (which has nothing at all to do with indentation). if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) -line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') + sig = indent_line + 2; Why is this adding 2 rather than 1? if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; @@ -1193,10 +1196,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 +
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 12:58 PM, karthik nayak karthik@gmail.com wrote: Hey Tanay, 1. Yes just getting used to git send-email now, should follow that from now 2. I thought it shouldn't be a part of the commit, so i put it after the last --- 3. I did have a thought on your lines also , but concluded to it being more advantageous, you might be right though Minor etiquette note: On this list, respond inline rather than top-posting. To understand why, look at how much scrolling up and down a person has to do to relate your points 1, 2, and 3 to review statements made by Tanay at the very bottom of the email. Nice to hear from you. Thanks -Karthik Nayak On Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra tanay...@gmail.com wrote: Karthik Nayak karthik.188 at gmail.com writes: In record_author_date() : Replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable indent_line. In parse_signed_commit() : Replace line + gpg_sig_header_len by skip_prefix(), which is saved in a new const char variable indent_line. In parse_gpg_output() : Replace buf + strlen(sigcheck_gpg_status[i].check + 1) by skip_prefix(), which is saved in already available variable found. Signed-off-by: Karthik Nayak karthik.188 at gmail.com --- commit.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c at at -1098,6 +1100,7 at at int parse_signed_commit(const unsigned char *sha1, char *buffer = read_sha1_file(sha1, type, size); int in_signature, saw_signature = -1; char *line, *tail; + const char *indent_line; if (!buffer || type != OBJ_COMMIT) goto cleanup; at at -,11 +1114,11 at at int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) - line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') + sig = indent_line + 2; if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; Hi, I was attempting the same microproject so I thought I would share some points that were told to me earlier .The mail subject should have a increasing order of submission numbers for each revision(for example here it should be [PATCH v2]). Also write the superfluous stuff which you have written in the bottom after ---(the three dashes after the signed off statement) . In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header variables are precomputed outside of the function, and also Junio said to prefer starts_with(), whenever skip_prefix() advantages are not so important in the context.So the replace may not be so advantageous ,I may be wrong in this case. Cheers, Tanay Abhra. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c @@ -1193,10 +1196,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); Add a space after the plus sign. + if(!found) { found = strstr(buf, sigcheck_gpg_status[i].check); 'found' is being computed again here, even though you already computed it just above via skip_prefix(). This seems pretty wasteful. Ignore this objection. I must have misread the code as I was composing the email. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak karthik@gmail.com wrote: diff --git a/commit.c b/commit.c index 6bf4fe0..71a03e3 100644 --- a/commit.c +++ b/commit.c @@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1, char *next = memchr(line, '\n', tail - line); next = next ? next + 1 : tail; + indent_line = skip_prefix(line, gpg_sig_header); Even stranger variable name for a GPG signature (which has nothing at all to do with indentation). if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) -line[gpg_sig_header_len] == ' ') - sig = line + gpg_sig_header_len + 1; + else if (indent_line indent_line[1] == ' ') Also, shouldn't this be checking *indent_line (or indent_line[0]) rather than indent_line[1]? + sig = indent_line + 2; Why is this adding 2 rather than 1? if (sig) { strbuf_add(signature, sig, next - sig); saw_signature = 1; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit.c: Replace starts_with() with skip_prefix()
Thanks for the submission. Some commentary below to expose you to the review process on this project... On Mon, Mar 3, 2014 at 2:47 AM, Karthik Nayak karthik@gmail.com wrote: Replace with skip_prefix(), which uses the inbuilt function strcmp() to compare. Explaining the purpose of the change is indeed important, however, this description misses the mark. See below. Other Places were this can be implemented: commit.c : line 1117 commit.c : line 1197 Bonus points if you actually follow through with this. Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/commit.c b/commit.c index 6bf4fe0..1e181cf 100644 --- a/commit.c +++ b/commit.c @@ -552,6 +552,7 @@ static void record_author_date(struct author_date_slab *author_date, char *buffer = NULL; struct ident_split ident; char *date_end; + char *flag_sp; /* stores return value of skip_prefix() */ It's not clear why this variable is needed. It's only assigned (below) but never referenced. Also, the name conveys little or no meaning. If you need a comment to explain the purpose of the variable, that's probably a good indication that it's poorly named. unsigned long date; if (!commit-buffer) { @@ -566,7 +567,7 @@ 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 )) { + if ((flag_sp = skip_prefix(buf, author )) == NULL) { Unfortunately, this change makes the code more difficult to read. Let's review the GSoC microproject: Rewrite commit.c:record_author_date() to use skip_prefix(). Are there other places in this file where skip_prefix() would be **more readable** than starts_with()? Note the part I **highlighted**. This is a good clue that use of skip_prefix() can be used to simplify the code. Examine record_author_date() more carefully and see if you can identify how to do so. if (!line_end[0] || line_end[1] == '\n') return; /* end of header */ continue; -- 1.9.0.138.g2de3478 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit.c: Replace starts_with() with skip_prefix()
Hello Eric, Thanks for Pointing out everything, i had a thorough look and fixed a couple of things. Here is an Updated Patch. - Removed unnecessary code and variables. - Replaced all instances of starts_with() with skip_prefix() Replace starts_with() with skip_prefix() for better reading purposes. Also to replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable buf_skipprefix. Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..e5dc2e2 100644 --- a/commit.c +++ b/commit.c @@ -552,6 +552,7 @@ static void record_author_date(struct author_date_slab *author_date, char *buffer = NULL; struct ident_split ident; char *date_end; + const char *buf_skipprefix; unsigned long date; if (!commit-buffer) { @@ -562,18 +563,20 @@ static void record_author_date(struct author_date_slab *author_date, return; } + buf_skipprefix = skip_prefix(buf, author ); + for (buf = commit-buffer ? commit-buffer : buffer; buf; buf = line_end + 1) { line_end = strchrnul(buf, '\n'); - if (!starts_with(buf, author )) { + if (!buf_skipprefix) { 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 ))) || + buf_skipprefix, + line_end - buf_skipprefix) || !ident.date_begin || !ident.date_end) goto fail_exit; /* malformed author line */ break; @@ -1113,7 +1116,7 @@ int parse_signed_commit(const unsigned char *sha1, next = next ? next + 1 : tail; if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) + else if (skip_prefix(line, gpg_sig_header) line[gpg_sig_header_len] == ' ') sig = line + gpg_sig_header_len + 1; if (sig) { @@ -1193,7 +1196,7 @@ 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)) { + if (skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) { /* At the very beginning of the buffer */ found = buf + strlen(sigcheck_gpg_status[i].check + 1); } else { -- 1.9.0.138.g2de3478 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit.c: Replace starts_with() with skip_prefix()
On Mon, Mar 3, 2014 at 10:23 AM, karthik nayak karthik@gmail.com wrote: Hello Eric, Thanks for Pointing out everything, i had a thorough look and fixed a couple of things. Here is an Updated Patch. - Removed unnecessary code and variables. - Replaced all instances of starts_with() with skip_prefix() This commentary is important for the on-going email discussion but does not belong in the commit message for the permanent project history, so place it below the --- line just under your sign-off. Explaining what changed since the last attempt, as you do here, is a good thing. To further ease the review process, supply a link to the previous attempt, like this [1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/243194 Replace starts_with() with skip_prefix() for better reading purposes. Also to replace buf + strlen(author ) by skip_prefix(), which is saved in a new const char variable buf_skipprefix. The second sentence is really the meat of this change, and not at all incidental as Also implies. You should use it (or a refinement of it) as your commit message. The first sentence doesn't particularly add much and can easily be dropped. Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/commit.c b/commit.c index 6bf4fe0..e5dc2e2 100644 --- a/commit.c +++ b/commit.c @@ -552,6 +552,7 @@ static void record_author_date(struct author_date_slab *author_date, char *buffer = NULL; struct ident_split ident; char *date_end; + const char *buf_skipprefix; Read this response by Junio [2] to another GSoC candidate which explains why this is a poor variable name and how to craft a better one. [2]: http://thread.gmane.org/gmane.comp.version-control.git/243231/focus=243259 unsigned long date; if (!commit-buffer) { @@ -562,18 +563,20 @@ static void record_author_date(struct author_date_slab *author_date, return; } + buf_skipprefix = skip_prefix(buf, author ); Unfortunately, your patch is badly whitespace-damaged as if it was pasted into the mailer and mangled. (Your first submission did not have this problem.) If you can, use git send-email to avoid such corruption. + for (buf = commit-buffer ? commit-buffer : buffer; buf; buf = line_end + 1) { line_end = strchrnul(buf, '\n'); - if (!starts_with(buf, author )) { + if (!buf_skipprefix) { 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 ))) || + buf_skipprefix, + line_end - buf_skipprefix) || Looks reasonable (sans whitespace damage). !ident.date_begin || !ident.date_end) goto fail_exit; /* malformed author line */ break; @@ -1113,7 +1116,7 @@ int parse_signed_commit(const unsigned char *sha1, next = next ? next + 1 : tail; if (in_signature line[0] == ' ') sig = line + 1; - else if (starts_with(line, gpg_sig_header) + else if (skip_prefix(line, gpg_sig_header) line[gpg_sig_header_len] == ' ') sig = line + gpg_sig_header_len + 1; if (sig) { @@ -1193,7 +1196,7 @@ 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)) { + if (skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) { /* At the very beginning of the buffer */ found = buf + strlen(sigcheck_gpg_status[i].check + 1); } else { For these two sets of changes, unless you are actually taking advantage of the return value of skip_prefix(), the mere mechanical replacement of starts_with() with skip_prefix() actually hurts readability. Examine each of these cases more carefully to determine whether skip_prefix() is in fact useful. If so, take advantage of it. If not, explain in the patch commentary why skip_prefix() doesn't help. -- 1.9.0.138.g2de3478 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] commit.c: Replace starts_with() with skip_prefix()
Replace with skip_prefix(), which uses the inbuilt function strcmp() to compare. Other Places were this can be implemented: commit.c : line 1117 commit.c : line 1197 Signed-off-by: Karthik Nayak karthik@gmail.com --- commit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/commit.c b/commit.c index 6bf4fe0..1e181cf 100644 --- a/commit.c +++ b/commit.c @@ -552,6 +552,7 @@ static void record_author_date(struct author_date_slab *author_date, char *buffer = NULL; struct ident_split ident; char *date_end; + char *flag_sp; /* stores return value of skip_prefix() */ unsigned long date; if (!commit-buffer) { @@ -566,7 +567,7 @@ 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 )) { + if ((flag_sp = skip_prefix(buf, author )) == NULL) { if (!line_end[0] || line_end[1] == '\n') return; /* end of header */ continue; -- 1.9.0.138.g2de3478 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html