Re: [PATCH 1/2] color: make it easier for non-config to parse color specs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 18, 2009 at 06:45:00PM +0100, René Scharfe wrote:

> > need a "memcasecmp" here.
> 
> 	if (!strncasecmp(value, "reset", len)) {

Thanks. I knew there must be some stock function to do this, but for
some reason I just could not think of strncasecmp.

>    die("bad color value '%.*s' for variable '%s', len, value, var);

Except that we've been shortening "len" during the course of the
function, so it is generally 0 at this point.

> >   $ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null
> >   real    0m0.673s
> >   user    0m0.652s
> >   sys     0m0.016s
> >   $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
> >   real    0m0.692s
> >   user    0m0.660s
> >   sys     0m0.032s
> > 
> > That's about 1 microsecond per commit.
> 
> Hmm, not too much overhead, agreed, but it would still be nice to avoid it.

Here are the numbers with your fixes:

  $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
  real    0m0.677s
  user    0m0.668s
  sys     0m0.008s

which puts the difference well into the noise region (I actually did get
one run with your patch that was just as fast as the original).

Here is an updated version of patch 1/2, squashing in your
color_mem_parse update, your follow-on fix, and a fix for the die().

-- >8 --
color: make it easier for non-config to parse color specs

We have very featureful color-parsing routines which are
used for color.diff.* and other options. Let's make it
easier to use those routines from other parts of the code.

This patch converts color_parse to color_parse_mem, taking a
length-bounded string instead of a NUL-terminated one. We
keep color_parse as a wrapper which takes a normal string.
Thanks to René Scharfe for rewriting the color_parse
implementation.

This also changes the error string for an invalid color not
to mention the word "config", since it is not always
appropriate (and when it is, the context is obvious since
the offending config variable is given).

Finally, while we are in the area, we clean up the parameter
names in the declaration of color_parse; the var and value
parameters were reversed from the actual implementation.
---
 color.c |   31 +++++++++++++++++++++----------
 color.h |    3 ++-
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/color.c b/color.c
index fc0b72a..915d7a9 100644
--- a/color.c
+++ b/color.c
@@ -41,29 +41,40 @@ static int parse_attr(const char *name, int len)
 
 void color_parse(const char *value, const char *var, char *dst)
 {
+	color_parse_mem(value, strlen(value), var, dst);
+}
+
+void color_parse_mem(const char *value, int value_len, const char *var,
+		char *dst)
+{
 	const char *ptr = value;
+	int len = value_len;
 	int attr = -1;
 	int fg = -2;
 	int bg = -2;
 
-	if (!strcasecmp(value, "reset")) {
+	if (!strncasecmp(value, "reset", len)) {
 		strcpy(dst, "\033[m");
 		return;
 	}
 
 	/* [fg [bg]] [attr] */
-	while (*ptr) {
+	while (len > 0) {
 		const char *word = ptr;
-		int val, len = 0;
+		int val, wordlen = 0;
 
-		while (word[len] && !isspace(word[len]))
-			len++;
+		while (len > 0 && !isspace(word[wordlen])) {
+			wordlen++;
+			len--;
+		}
 
-		ptr = word + len;
-		while (*ptr && isspace(*ptr))
+		ptr = word + wordlen;
+		while (len > 0 && isspace(*ptr)) {
 			ptr++;
+			len--;
+		}
 
-		val = parse_color(word, len);
+		val = parse_color(word, wordlen);
 		if (val >= -1) {
 			if (fg == -2) {
 				fg = val;
@@ -75,7 +86,7 @@ void color_parse(const char *value, const char *var, char *dst)
 			}
 			goto bad;
 		}
-		val = parse_attr(word, len);
+		val = parse_attr(word, wordlen);
 		if (val < 0 || attr != -1)
 			goto bad;
 		attr = val;
@@ -115,7 +126,7 @@ void color_parse(const char *value, const char *var, char *dst)
 	*dst = 0;
 	return;
 bad:
-	die("bad config value '%s' for variable '%s'", value, var);
+	die("bad color value '%.*s' for variable '%s'", value_len, value, var);
 }
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
diff --git a/color.h b/color.h
index 6cf5c88..7066099 100644
--- a/color.h
+++ b/color.h
@@ -16,7 +16,8 @@ extern int git_use_color_default;
 int git_color_default_config(const char *var, const char *value, void *cb);
 
 int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
-void color_parse(const char *var, const char *value, char *dst);
+void color_parse(const char *value, const char *var, char *dst);
+void color_parse_mem(const char *value, int len, const char *var, char *dst);
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
 
-- 
1.6.1.266.g3b9d0.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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux