[PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()

2014-03-04 Thread Karthik Nayak
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()

2014-03-04 Thread Tanay Abhra

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()

2014-03-04 Thread karthik nayak
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()

2014-03-04 Thread Eric Sunshine
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()

2014-03-04 Thread Eric Sunshine
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()

2014-03-04 Thread Eric Sunshine
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()

2014-03-04 Thread Eric Sunshine
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