Haven't tested, just eyeballing the diff; I have two questions inline below. (Apologies if iOS Mail.app mangles the quoting :()
> On Aug 14, 2014, at 17:39, Vadim Zhukov <[email protected]> wrote: > > 2014-08-11 4:37 GMT+04:00 Vadim Zhukov <[email protected]>: >> This patch adds support for handling commit IDs in loginfo scripts. >> Now, if you add, e.g., the following line to loginfo... >> >> date %{isVv} >>/tmp/commitlog >> >> ... then you'll get something like: >> >> DOQ1DVGdv6tda71v ports/x11/kde4/libs TODO,1.14,1.15 >> >> As a bonus, this adds support for '%%' escaping, so now you can >> use, say, "date +%%s; echo %s" in loginfo, too. >> >> I think this should simplify handling commit messages as a single >> batch a lot. Any comments? > > Okay/deny anyone? > > >> Index: src/logmsg.c >> =================================================================== >> RCS file: /cvs/src/gnu/usr.bin/cvs/src/logmsg.c,v >> retrieving revision 1.4 >> diff -u -p -r1.4 logmsg.c >> --- src/logmsg.c 4 Mar 2012 04:05:15 -0000 1.4 >> +++ src/logmsg.c 10 Aug 2014 23:29:50 -0000 >> @@ -584,6 +584,9 @@ title_proc (p, closure) >> { >> switch (*c) >> { >> + case 'i': >> + /* handled above */ >> + continue; >> case 's': >> str_list = >> xrealloc (str_list, >> @@ -677,6 +680,7 @@ logfile_write (repository, filter, messa >> or followed by a set of format characters surrounded by `{' and >> `}' as separators. The format characters are: >> >> + i = commit ID >> s = file name >> t = tag name >> V = old version number (pre-checkin) >> @@ -685,6 +689,7 @@ logfile_write (repository, filter, messa >> For example, valid format strings are: >> >> %{} >> + %i >> %s >> %{s} >> %{sVv} >> @@ -709,10 +714,12 @@ logfile_write (repository, filter, messa >> Why this duplicates the old behavior when the format string is >> `%s' is left as an exercise for the reader. */ >> >> - fmt_percent = strchr (filter, '%'); >> + fmt_percent = filter; >> +find_percent: >> + fmt_percent = strchr (fmt_percent, '%'); >> if (fmt_percent) >> { >> - int len; >> + int len, proglen; >> char *srepos; >> char *fmt_begin, *fmt_end; /* beginning and end of the >> format string specified in >> @@ -737,6 +744,12 @@ logfile_write (repository, filter, messa >> fmt_end = fmt_begin; >> fmt_continue = fmt_begin; >> } >> + else if (*(fmt_percent + 1) == '%') >> + { >> + memmove(fmt_percent, fmt_percent+1, strlen(fmt_percent)); >> + fmt_percent++; >> + goto find_percent; >> + } >> else if (*(fmt_percent + 1) == '{') >> { >> /* The percent has a set of characters following it. */ >> @@ -762,8 +775,7 @@ logfile_write (repository, filter, messa >> } >> else >> { >> - /* The percent has a single character following it. FIXME: >> - %% should expand to a regular percent sign. */ >> + /* The percent has a single character following it. */ >> >> fmt_begin = fmt_percent + 1; >> fmt_end = fmt_begin + 1; >> @@ -795,19 +807,28 @@ logfile_write (repository, filter, messa >> type = T_REMOVED; >> (void) walklist (changes, title_proc, NULL); >> } >> - >> - free (str_list_format); >> >> /* Construct the final string. */ >> >> srepos = Short_Repository (repository); >> >> + proglen = (fmt_percent - filter) + 2 * strlen (srepos) >> + + 2 * strlen (str_list) + strlen (fmt_continue) >> + + 10; >> + if (strchr (str_list_format, 'i') != NULL) >> + /* assume no escaping needed */ >> + proglen += strlen (global_session_id) * 2 + 1; >> + >> prog = cp = xmalloc ((fmt_percent - filter) + 2 * strlen (srepos) >> + 2 * strlen (str_list) + strlen (fmt_continue) >> + 10); 1). Did you want to use proglen here? 2). proglen is declared as an int; is there any danger malicious values could cause an overflow, or are all of those values carefully controlled? >> (void) memcpy (cp, filter, fmt_percent - filter); >> cp += fmt_percent - filter; >> *cp++ = '"'; >> + if (strchr (str_list_format, 'i') != NULL) { >> + cp = shell_escape (cp, global_session_id); >> + *cp++ = ' '; >> + } >> cp = shell_escape (cp, srepos); >> cp = shell_escape (cp, str_list); >> *cp++ = '"'; >> @@ -815,6 +836,7 @@ logfile_write (repository, filter, messa >> >> /* To be nice, free up some memory. */ >> >> + free (str_list_format); >> free (str_list); >> str_list = (char *) NULL; >> } >> Index: src/mkmodules.c >> =================================================================== >> RCS file: /cvs/src/gnu/usr.bin/cvs/src/mkmodules.c,v >> retrieving revision 1.11 >> diff -u -p -r1.11 mkmodules.c >> --- src/mkmodules.c 4 Mar 2012 04:05:15 -0000 1.11 >> +++ src/mkmodules.c 10 Aug 2014 23:29:50 -0000 >> @@ -65,6 +65,7 @@ static const char *const loginfo_content >> "# characters surrounded by `{' and `}' as separators. The format\n", >> "# characters are:\n", >> "#\n", >> + "# i = commit ID\n", >> "# s = file name\n", >> "# t = tag name\n", >> "# V = old version number (pre-checkin)\n", >> @@ -73,7 +74,7 @@ static const char *const loginfo_content >> "# For example:\n", >> "#DEFAULT (echo \"\"; id; echo %s; date; cat) >> >> $CVSROOT/CVSROOT/commitlog\n", >> "# or\n", >> - "#DEFAULT (echo \"\"; id; echo %{sVv}; date; cat) >> >> $CVSROOT/CVSROOT/commitlog\n", >> + "#DEFAULT (echo \"\"; id; echo %{isVv}; date; cat) >> >> $CVSROOT/CVSROOT/commitlog\n", >> NULL >> }; > > -- > WBR, > Vadim Zhukov >
