Re: [PATCH] dir.c: avoid c99 array initialization

"Shawn O. Pearce" [off-list ref] writes:

Alex Riesen [off-list ref] wrote:

quoted

For just these 5 values it is likely more effective to just use
a conditional statement (less stack requested, less likely
some stupid compiler tries to optimize it wrongly).
And just as readable.

diff --git a/dir.c b/dir.c
index 92452eb..1cf5985 100644
--- a/dir.c
+++ b/dir.c

@@ -680,17 +680,12 @@ static int cmp_name(const void *p1, const void *p2)
  */
 static int simple_length(const char *match)
 {
-	const char special[256] = {
-		[0] = 1, ['?'] = 1,
-		['\\'] = 1, ['*'] = 1,
-		['['] = 1
-	};
 	int len = -1;

 	for (;;) {
 		unsigned char c = *match++;
 		len++;
-		if (special[c])
+		if (!c || '?' == c || '\\' == c || '*' == c || '[' == c)

I am reminded of a year old thread with my patch to this:

  http://kerneltrap.org/mailarchive/git/2007/4/15/243541

The patch never applied.  I wonder why.  Was it just Dscho's comment?

I think it was an unanswered question about stealing one bit from
ctype.c::sane_ctype[] that kept the discussion in limbo.

The following syntax:

        char foo[] = {
                [0] = 1,
                [7] = 2,
                [15] = 3
        };

is a c99 construct which some compilers do not support even though they
support other c99 constructs. This construct can be avoided by folding
these 'special' test cases into the sane_ctype array and making use of
the related infrastructure.

Signed-off-by: Brandon Casey <redacted>
---


Junio C Hamano wrote:

"Shawn O. Pearce" [off-list ref] writes:

quoted

quoted

I am reminded of a year old thread with my patch to this:

  http://kerneltrap.org/mailarchive/git/2007/4/15/243541

The patch never applied.  I wonder why.  Was it just Dscho's comment?

I think it was an unanswered question about stealing one bit from
ctype.c::sane_ctype[] that kept the discussion in limbo.

That seems easy enough. Anybody got a 5 character abbreviation for special?

-brandon


 ctype.c           |   10 ++++++----
 dir.c             |    7 +------
 git-compat-util.h |   16 +++++++++-------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/ctype.c b/ctype.c
index d2bd38e..acb5f0f 100644
--- a/ctype.c
+++ b/ctype.c

@@ -9,18 +9,20 @@
 #undef SS
 #undef AA
 #undef DD
+#undef GS
 
 #define SS GIT_SPACE
 #define AA GIT_ALPHA
 #define DD GIT_DIGIT
+#define GS GIT_SPECIAL  /* \0, *, ?, [, \\ */
 
 unsigned char sane_ctype[256] = {
-	 0,  0,  0,  0,  0,  0,  0,  0,  0, SS, SS,  0,  0, SS,  0,  0,		/* 0-15 */
+	GS,  0,  0,  0,  0,  0,  0,  0,  0, SS, SS,  0,  0, SS,  0,  0,		/* 0-15 */
 	 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,		/* 16-15 */
-	SS,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,		/* 32-15 */
-	DD, DD, DD, DD, DD, DD, DD, DD, DD, DD,  0,  0,  0,  0,  0,  0,		/* 48-15 */
+	SS,  0,  0,  0,  0,  0,  0,  0,  0,  0, GS,  0,  0,  0,  0,  0,		/* 32-15 */
+	DD, DD, DD, DD, DD, DD, DD, DD, DD, DD,  0,  0,  0,  0,  0, GS,		/* 48-15 */
 	 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,		/* 64-15 */
-	AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,  0,  0,  0,  0,  0,		/* 80-15 */
+	AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, GS, GS,  0,  0,  0,		/* 80-15 */
 	 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,		/* 96-15 */
 	AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,  0,  0,  0,  0,  0,		/* 112-15 */
 	/* Nothing in the 128.. range */

diff --git a/dir.c b/dir.c
index 92452eb..e6a2556 100644
--- a/dir.c
+++ b/dir.c

@@ -680,17 +680,12 @@ static int cmp_name(const void *p1, const void *p2)
  */
 static int simple_length(const char *match)
 {
-	const char special[256] = {
-		[0] = 1, ['?'] = 1,
-		['\\'] = 1, ['*'] = 1,
-		['['] = 1
-	};
 	int len = -1;
 
 	for (;;) {
 		unsigned char c = *match++;
 		len++;
-		if (special[c])
+		if (isspecial(c))
 			return len;
 	}
 }

diff --git a/git-compat-util.h b/git-compat-util.h
index 6ee3255..ac22409 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -326,14 +326,16 @@ static inline int has_extension(const char *filename, const char *ext)
 #undef tolower
 #undef toupper
 extern unsigned char sane_ctype[256];
-#define GIT_SPACE 0x01
-#define GIT_DIGIT 0x02
-#define GIT_ALPHA 0x04
+#define GIT_SPACE   0x01
+#define GIT_DIGIT   0x02
+#define GIT_ALPHA   0x04
+#define GIT_SPECIAL 0x08
 #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
-#define isspace(x) sane_istest(x,GIT_SPACE)
-#define isdigit(x) sane_istest(x,GIT_DIGIT)
-#define isalpha(x) sane_istest(x,GIT_ALPHA)
-#define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
+#define isspace(x)   sane_istest(x,GIT_SPACE)
+#define isdigit(x)   sane_istest(x,GIT_DIGIT)
+#define isalpha(x)   sane_istest(x,GIT_ALPHA)
+#define isalnum(x)   sane_istest(x,GIT_ALPHA | GIT_DIGIT)
+#define isspecial(x) sane_istest(x,GIT_SPECIAL)
 #define tolower(x) sane_case((unsigned char)(x), 0x20)
 #define toupper(x) sane_case((unsigned char)(x), 0)
 

-- 
1.6.0.21.g35a2e

2008/8/29 Brandon Casey [off-list ref]:

quoted

I think it was an unanswered question about stealing one bit from
ctype.c::sane_ctype[] that kept the discussion in limbo.

That seems easy enough. Anybody got a 5 character abbreviation for special?

SPECL :)

Brandon Casey wrote:

The following syntax:

        char foo[] = {
                [0] = 1,
                [7] = 2,
                [15] = 3
        };

is a c99 construct which some compilers do not support even though they
support other c99 constructs. This construct can be avoided by folding
these 'special' test cases into the sane_ctype array and making use of
the related infrastructure.

You probably would want to squash this in:

diff --git a/dir.c b/dir.c
index e6a2556..acf1001 100644
--- a/dir.c
+++ b/dir.c

@@ -52,11 +52,6 @@ int common_prefix(const char **pathspec)

	return prefix;
 }
 
-static inline int special_char(unsigned char c1)
-{
-	return !c1 || c1 == '*' || c1 == '[' || c1 == '?' || c1 == '\\';
-}
-
 /*
  * Does 'match' matches the given name?
  * A match is found if

@@ -80,7 +75,7 @@ static int match_one(const char *match, const char *name, int 
 	for (;;) {
 		unsigned char c1 = *match;
 		unsigned char c2 = *name;
-		if (special_char(c1))
+		if (isspecial(c1))
 			break;
 		if (c1 != c2)
 			return 0;