Skip to content

Commit 9ff700e

Browse files
gitstergitster
committed
Merge branch 'jk/commit-author-parsing'
Code clean-up. * jk/commit-author-parsing: determine_author_info(): copy getenv output determine_author_info(): reuse parsing functions date: use strbufs in date-formatting functions record_author_date(): use find_commit_header() record_author_date(): fix memory leak on malformed commit commit: provide a function to find a header in a buffer
2 parents ceeacc5 + f4ef517 commit 9ff700e

File tree

9 files changed

+130
-120
lines changed

9 files changed

+130
-120
lines changed

‎builtin/commit.c‎

Lines changed: 48 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -545,77 +545,80 @@ static int sane_ident_split(struct ident_split *person)
545545
return 1;
546546
}
547547

548-
static int parse_force_date(const char *in, char *out, int len)
548+
static int parse_force_date(const char *in, struct strbuf *out)
549549
{
550-
if (len < 1)
551-
return -1;
552-
*out++ = '@';
553-
len--;
550+
strbuf_addch(out, '@');
554551

555-
if (parse_date(in, out, len) < 0) {
552+
if (parse_date(in, out) < 0) {
556553
int errors = 0;
557554
unsigned long t = approxidate_careful(in, &errors);
558555
if (errors)
559556
return -1;
560-
snprintf(out, len, "%lu", t);
557+
strbuf_addf(out, "%lu", t);
561558
}
562559

563560
return 0;
564561
}
565562

563+
static void set_ident_var(char **buf, char *val)
564+
{
565+
free(*buf);
566+
*buf = val;
567+
}
568+
569+
static char *envdup(const char *var)
570+
{
571+
const char *val = getenv(var);
572+
return val ? xstrdup(val) : NULL;
573+
}
574+
566575
static void determine_author_info(struct strbuf *author_ident)
567576
{
568577
char *name, *email, *date;
569578
struct ident_split author;
570-
char date_buf[64];
571579

572-
name = getenv("GIT_AUTHOR_NAME");
573-
email = getenv("GIT_AUTHOR_EMAIL");
574-
date = getenv("GIT_AUTHOR_DATE");
580+
name = envdup("GIT_AUTHOR_NAME");
581+
email = envdup("GIT_AUTHOR_EMAIL");
582+
date = envdup("GIT_AUTHOR_DATE");
575583

576584
if (author_message) {
577-
const char *a, *lb, *rb, *eol;
585+
struct ident_split ident;
578586
size_t len;
587+
const char *a;
579588

580-
a = strstr(author_message_buffer, "\nauthor ");
589+
a = find_commit_header(author_message_buffer, "author", &len);
581590
if (!a)
582-
die(_("invalid commit: %s"), author_message);
583-
584-
lb = strchrnul(a + strlen("\nauthor "), '<');
585-
rb = strchrnul(lb, '>');
586-
eol = strchrnul(rb, '\n');
587-
if (!*lb || !*rb || !*eol)
588-
die(_("invalid commit: %s"), author_message);
589-
590-
if (lb == a + strlen("\nauthor "))
591-
/* \nauthor <foo@example.com> */
592-
name = xcalloc(1, 1);
593-
else
594-
name = xmemdupz(a + strlen("\nauthor "),
595-
(lb - strlen(" ") -
596-
(a + strlen("\nauthor "))));
597-
email = xmemdupz(lb + strlen("<"), rb - (lb + strlen("<")));
598-
len = eol - (rb + strlen("> "));
599-
date = xmalloc(len + 2);
600-
*date = '@';
601-
memcpy(date + 1, rb + strlen("> "), len);
602-
date[len + 1] = '\0';
591+
die(_("commit '%s' lacks author header"), author_message);
592+
if (split_ident_line(&ident, a, len) < 0)
593+
die(_("commit '%s' has malformed author line"), author_message);
594+
595+
set_ident_var(&name, xmemdupz(ident.name_begin, ident.name_end - ident.name_begin));
596+
set_ident_var(&email, xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin));
597+
598+
if (ident.date_begin) {
599+
struct strbuf date_buf = STRBUF_INIT;
600+
strbuf_addch(&date_buf, '@');
601+
strbuf_add(&date_buf, ident.date_begin, ident.date_end - ident.date_begin);
602+
strbuf_addch(&date_buf, ' ');
603+
strbuf_add(&date_buf, ident.tz_begin, ident.tz_end - ident.tz_begin);
604+
set_ident_var(&date, strbuf_detach(&date_buf, NULL));
605+
}
603606
}
604607

605608
if (force_author) {
606-
const char *lb = strstr(force_author, " <");
607-
const char *rb = strchr(force_author, '>');
609+
struct ident_split ident;
608610

609-
if (!lb || !rb)
611+
if (split_ident_line(&ident, force_author, strlen(force_author)) < 0)
610612
die(_("malformed --author parameter"));
611-
name = xstrndup(force_author, lb - force_author);
612-
email = xstrndup(lb + 2, rb - (lb + 2));
613+
set_ident_var(&name, xmemdupz(ident.name_begin, ident.name_end - ident.name_begin));
614+
set_ident_var(&email, xmemdupz(ident.mail_begin, ident.mail_end - ident.mail_begin));
613615
}
614616

615617
if (force_date) {
616-
if (parse_force_date(force_date, date_buf, sizeof(date_buf)))
618+
struct strbuf date_buf = STRBUF_INIT;
619+
if (parse_force_date(force_date, &date_buf))
617620
die(_("invalid date format: %s"), force_date);
618-
date = date_buf;
621+
set_ident_var(&date, strbuf_detach(&date_buf, NULL));
619622
}
620623

621624
strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT));
@@ -625,6 +628,10 @@ static void determine_author_info(struct strbuf *author_ident)
625628
export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0);
626629
export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@');
627630
}
631+
632+
free(name);
633+
free(email);
634+
free(date);
628635
}
629636

630637
static void split_ident_or_die(struct ident_split *id, const struct strbuf *buf)

‎cache.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,10 +1047,10 @@ enum date_mode {
10471047
const char *show_date(unsigned long time, int timezone, enum date_mode mode);
10481048
void show_date_relative(unsigned long time, int tz, const struct timeval *now,
10491049
struct strbuf *timebuf);
1050-
int parse_date(const char *date, char *buf, int bufsize);
1050+
int parse_date(const char *date, struct strbuf *out);
10511051
int parse_date_basic(const char *date, unsigned long *timestamp, int *offset);
10521052
int parse_expiry_date(const char *date, unsigned long *timestamp);
1053-
void datestamp(char *buf, int bufsize);
1053+
void datestamp(struct strbuf *out);
10541054
#define approxidate(s) approxidate_careful((s), NULL)
10551055
unsigned long approxidate_careful(const char *, int *);
10561056
unsigned long approxidate_relative(const char *date, const struct timeval *now);

‎commit.c‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -584,25 +584,19 @@ define_commit_slab(author_date_slab, unsigned long);
584584
static void record_author_date(struct author_date_slab *author_date,
585585
struct commit *commit)
586586
{
587-
const char *buf, *line_end, *ident_line;
588587
const char *buffer = get_commit_buffer(commit, NULL);
589588
struct ident_split ident;
589+
const char *ident_line;
590+
size_t ident_len;
590591
char *date_end;
591592
unsigned long date;
592593

593-
for (buf = buffer; buf; buf = line_end + 1) {
594-
line_end = strchrnul(buf, '\n');
595-
if (!skip_prefix(buf, "author ", &ident_line)) {
596-
if (!line_end[0] || line_end[1] == '\n')
597-
return; /* end of header */
598-
continue;
599-
}
600-
if (split_ident_line(&ident,
601-
ident_line, line_end - ident_line) ||
602-
!ident.date_begin || !ident.date_end)
603-
goto fail_exit; /* malformed "author" line */
604-
break;
605-
}
594+
ident_line = find_commit_header(buffer, "author", &ident_len);
595+
if (!ident_line)
596+
goto fail_exit; /* no author line */
597+
if (split_ident_line(&ident, ident_line, ident_len) ||
598+
!ident.date_begin || !ident.date_end)
599+
goto fail_exit; /* malformed "author" line */
606600

607601
date = strtoul(ident.date_begin, &date_end, 10);
608602
if (date_end != ident.date_end)
@@ -1660,3 +1654,25 @@ void print_commit_list(struct commit_list *list,
16601654
printf(format, sha1_to_hex(list->item->object.sha1));
16611655
}
16621656
}
1657+
1658+
const char *find_commit_header(const char *msg, const char *key, size_t *out_len)
1659+
{
1660+
int key_len = strlen(key);
1661+
const char *line = msg;
1662+
1663+
while (line) {
1664+
const char *eol = strchrnul(line, '\n');
1665+
1666+
if (line == eol)
1667+
return NULL;
1668+
1669+
if (eol - line > key_len &&
1670+
!strncmp(line, key, key_len) &&
1671+
line[key_len] == ' ') {
1672+
*out_len = eol - line - key_len - 1;
1673+
return line + key_len + 1;
1674+
}
1675+
line = *eol ? eol + 1 : NULL;
1676+
}
1677+
return NULL;
1678+
}

‎commit.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,17 @@ extern struct commit_extra_header *read_commit_extra_headers(struct commit *, co
326326

327327
extern void free_commit_extra_headers(struct commit_extra_header *extra);
328328

329+
/*
330+
* Search the commit object contents given by "msg" for the header "key".
331+
* Returns a pointer to the start of the header contents, or NULL. The length
332+
* of the header, up to the first newline, is returned via out_len.
333+
*
334+
* Note that some headers (like mergetag) may be multi-line. It is the caller's
335+
* responsibility to parse further in this case!
336+
*/
337+
extern const char *find_commit_header(const char *msg, const char *key,
338+
size_t *out_len);
339+
329340
typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
330341
void *cb_data);
331342

‎date.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -614,15 +614,15 @@ static int match_tz(const char *date, int *offp)
614614
return end - date;
615615
}
616616

617-
static int date_string(unsigned long date, int offset, char *buf, int len)
617+
static void date_string(unsigned long date, int offset, struct strbuf *buf)
618618
{
619619
int sign = '+';
620620

621621
if (offset < 0) {
622622
offset = -offset;
623623
sign = '-';
624624
}
625-
return snprintf(buf, len, "%lu %c%02d%02d", date, sign, offset / 60, offset % 60);
625+
strbuf_addf(buf, "%lu %c%02d%02d", date, sign, offset / 60, offset % 60);
626626
}
627627

628628
/*
@@ -744,13 +744,14 @@ int parse_expiry_date(const char *date, unsigned long *timestamp)
744744
return errors;
745745
}
746746

747-
int parse_date(const char *date, char *result, int maxlen)
747+
int parse_date(const char *date, struct strbuf *result)
748748
{
749749
unsigned long timestamp;
750750
int offset;
751751
if (parse_date_basic(date, &timestamp, &offset))
752752
return -1;
753-
return date_string(timestamp, offset, result, maxlen);
753+
date_string(timestamp, offset, result);
754+
return 0;
754755
}
755756

756757
enum date_mode parse_date_format(const char *format)
@@ -778,7 +779,7 @@ enum date_mode parse_date_format(const char *format)
778779
die("unknown date format %s", format);
779780
}
780781

781-
void datestamp(char *buf, int bufsize)
782+
void datestamp(struct strbuf *out)
782783
{
783784
time_t now;
784785
int offset;
@@ -788,7 +789,7 @@ void datestamp(char *buf, int bufsize)
788789
offset = tm_to_time_t(localtime(&now)) - now;
789790
offset /= 60;
790791

791-
date_string(now, offset, buf, bufsize);
792+
date_string(now, offset, out);
792793
}
793794

794795
/*

‎fast-import.c‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,7 +1996,7 @@ static int parse_data(struct strbuf *sb, uintmax_t limit, uintmax_t *len_res)
19961996
return 1;
19971997
}
19981998

1999-
static int validate_raw_date(const char *src, char *result, int maxlen)
1999+
static int validate_raw_date(const char *src, struct strbuf *result)
20002000
{
20012001
const char *orig_src = src;
20022002
char *endp;
@@ -2014,19 +2014,18 @@ static int validate_raw_date(const char *src, char *result, int maxlen)
20142014
return -1;
20152015

20162016
num = strtoul(src + 1, &endp, 10);
2017-
if (errno || endp == src + 1 || *endp || (endp - orig_src) >= maxlen ||
2018-
1400 < num)
2017+
if (errno || endp == src + 1 || *endp || 1400 < num)
20192018
return -1;
20202019

2021-
strcpy(result, orig_src);
2020+
strbuf_addstr(result, orig_src);
20222021
return 0;
20232022
}
20242023

20252024
static char *parse_ident(const char *buf)
20262025
{
20272026
const char *ltgt;
20282027
size_t name_len;
2029-
char *ident;
2028+
struct strbuf ident = STRBUF_INIT;
20302029

20312030
/* ensure there is a space delimiter even if there is no name */
20322031
if (*buf == '<')
@@ -2045,26 +2044,25 @@ static char *parse_ident(const char *buf)
20452044
die("Missing space after > in ident string: %s", buf);
20462045
ltgt++;
20472046
name_len = ltgt - buf;
2048-
ident = xmalloc(name_len + 24);
2049-
strncpy(ident, buf, name_len);
2047+
strbuf_add(&ident, buf, name_len);
20502048

20512049
switch (whenspec) {
20522050
case WHENSPEC_RAW:
2053-
if (validate_raw_date(ltgt, ident + name_len, 24) < 0)
2051+
if (validate_raw_date(ltgt, &ident) < 0)
20542052
die("Invalid raw date \"%s\" in ident: %s", ltgt, buf);
20552053
break;
20562054
case WHENSPEC_RFC2822:
2057-
if (parse_date(ltgt, ident + name_len, 24) < 0)
2055+
if (parse_date(ltgt, &ident) < 0)
20582056
die("Invalid rfc2822 date \"%s\" in ident: %s", ltgt, buf);
20592057
break;
20602058
case WHENSPEC_NOW:
20612059
if (strcmp("now", ltgt))
20622060
die("Date in ident must be 'now': %s", buf);
2063-
datestamp(ident + name_len, 24);
2061+
datestamp(&ident);
20642062
break;
20652063
}
20662064

2067-
return ident;
2065+
return strbuf_detach(&ident, NULL);
20682066
}
20692067

20702068
static void parse_and_store_blob(

0 commit comments

Comments
 (0)