Hi,
Brandon Casey wrote:
> --- a/sequencer.c
> +++ b/sequencer.c
[...]
> @@ -1042,13 +1041,8 @@ static int ends_rfc2822_footer(struct strbuf *sb, int
> ignore_footer)
Git is checking if (sb->buf) ends with a "Signed-off-by:" style
line. If it doesn't, it will need to add an extra blank line
before adding a new sign-off.
First (snipped), it seeks back two newlines from the end and then
forward to the next non-newline character, so (buf + i) is at the
start of the last line of (the interesting part of) sb. Now:
> for (; i < len; i = k) {
> for (k = i; k < len && buf[k] != '\n'; k++)
> ; /* do nothing */
> k++;
(buf + k) points to the end of this line.
> - if ((buf[k] == ' ' || buf[k] == '\t') && !first)
> - continue;
This is always the first line examined, so this "continue" never
triggers.
> -
> - first = 0;
> -
> for (j = 0; i + j < len; j++) {
If the line matches /^[[:alnum:]-]*:/, it passes and git moves on to
the (nonexistent) next line. Otherwise, it fails.
Do I understand correctly? If so, this patch should be a no-op, which
is good, I guess.
But in that case, couldn't this function be made much simpler? As far
as I can tell, all the function needs to do is the following:
1. Find the last line.
2. Check if it is blank or matches /^[[:alnum:]-]*:/
3. There is no step 3. That's it.
In other words, something like:
const char *eol, *p;
/* End of line */
eol = memrchr(sb->buf, '\n', sb->len - ignore_footer);
if (!eol)
eol = sb->buf;
/* Start of line */
p = memrchr(sb->buf, '\n', eol - sb->buf);
if (p)
p++;
else
p = sb->buf;
if (p == eol) /* Blank line? */
return 1;
/* "Signed-off-by"-style field */
while ((isalnum(*p) || *p == '-') && p < eol)
p++;
return *p == ':';
where memrchr is defined roughly as follows[1]:
#ifdef __GLIBC_PREREQ
#if __GLIBC_PREREQ(2, 2)
#define HAVE_MEMRCHR
#endif
#endif
#ifndef HAVE_MEMRCHR
#define memrchr gitmemrchr
static inline void *gitmemrchr(const void *s, int c, size_t n)
{
const unsigned char *p = s;
p += n;
while (p != s)
if (*--p == (unsigned char) c)
return p;
return NULL;
}
#endif
Does that look right?
Jonathan
[1] http://thread.gmane.org/gmane.comp.version-control.git/159081/focus=159121
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
