Re: [PATCH v2 1/2] bugfix: fix broken time_buf paddings for git-blame
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Apr 20, 2014, 20:28 UTC
- Message-ID
- <CAPig+cSf8RcUBdeACSLTTNcF7hqCXX-jjxx7RDZt52u3BKJxDA@mail.gmail.com>
- In-Reply-To
- <2742234ee9199b26e646f30b6ae20a1c9bfe48f7.1398010052.git.worldhello.net@gmail.com>
On Sun, Apr 20, 2014 at 12:13 PM, Jiang Xin <worldhello.net@gmail.com> 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/
Show 6 quoted lines
> 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/
Show 5 quoted lines
> 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/
Show 56 quoted lines
> `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@gmail.com>
> ---
> 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