[PATCH v2 1/2] bugfix: fix broken time_buf paddings for git-blame

2014-04-20 Thread Jiang Xin 
When `git blame` shows date field in a fixed width (as long as
blame_date_width characters), if time_str shorter than that, add spaces
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
   is greater than blame_ate_width, blame_date_width - time_len will
   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
   `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@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] bugfix: fix broken time_buf paddings for git-blame

2014-04-20 Thread Eric Sunshine 
On Sun, Apr 20, 2014 at 12:13 PM, Jiang Xin worldhello@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/

 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@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html