On Sun, Apr 20, 2014 at 12:13 PM, Jiang Xin <worldhello.net@xxxxxxxxx> wrote: > When `git blame` shows date field in a fixed width (as long as s/fixed width/fixed-width/ s/long/wide/ would read a bit better. > blame_date_width characters), if time_str shorter than that, add spaces s/shorter/is shorter/ s/add/it adds/ > for padding. But there are two bugs in the following codes: > > memcpy(time_buf, time_str, time_len); > memset(time_buf + time_len, ' ', blame_date_width - time_len); > > 1. The type of blame_date_width is size_t (unsigned int). If time_len s/(unsigned int)/, which is unsigned/ > is greater than blame_ate_width, "blame_date_width - time_len" will s/_ate/_date/ > never be a negative number, but a really big positive number, and > will cause memory overwrite. > > This bug can be triggered if either l10n message for function > show_date_relative() in date.c is longer then 30 charcters, then s/then 30/than 30/ s/charcters/characters/ > `git blame --date relative` may fail. > > 2. When show blame information with relative time, the UTF-8 characters > in time_str will break the alignment of columns after the date field. > This is because the time_buf padding with spaces should have a > constant display width, not a fixed strlen size. So we should call > utf8_strwidth() instead of strlen() for calibration. > > Signed-off-by: Jiang Xin <worldhello.net@xxxxxxxxx> > --- > builtin/blame.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/builtin/blame.c b/builtin/blame.c > index 88cb799..0a0a858 100644 > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -1556,22 +1556,31 @@ static void assign_blame(struct scoreboard *sb, int opt) > static const char *format_time(unsigned long time, const char *tz_str, > int show_raw_time) > { > - static char time_buf[128]; > + static struct strbuf time_buf = STRBUF_INIT; > > + strbuf_reset(&time_buf); > if (show_raw_time) { > - snprintf(time_buf, sizeof(time_buf), "%lu %s", time, tz_str); > + strbuf_addf(&time_buf, "%lu %s", time, tz_str); > } > else { > const char *time_str; > + size_t time_width; > int time_len; > int tz; > tz = atoi(tz_str); > time_str = show_date(time, tz, blame_date_mode); > time_len = strlen(time_str); > - memcpy(time_buf, time_str, time_len); > - memset(time_buf + time_len, ' ', blame_date_width - time_len); > + strbuf_add(&time_buf, time_str, time_len); > + /* > + * Add space paddings to time_buf to display a fixed width > + * string, and use time_width for display width calibration. > + */ > + for (time_width = utf8_strwidth(time_str); > + time_width < blame_date_width; > + time_width++) > + strbuf_addch(&time_buf, ' '); > } > - return time_buf; > + return time_buf.buf; > } > > #define OUTPUT_ANNOTATE_COMPAT 001 > -- > 2.0.0.rc0.3.g444188f.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html