Re: [PATCH 1/7] Remove ranges from switch statements.

Re: [PATCH 1/7] Remove ranges from switch statements.

[linux]

> Yes, but isalnum(ch) even better ;)

And if you want to get fancy, there's plenty of room in git's sane_ctype
array for an additional punctuation bit to catch the [./-] cases.

Note also that technically the set of unreserved characters in URIs
(according to rfc2396 section 2.3) is

      unreserved  = alphanum | mark

      mark        = "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"

The complementary set of characters that should be quoted is defined as well:

(2.2)
   Many URI include components consisting of or delimited by, certain
   special characters.  These characters are called "reserved", since
   their usage within the URI component is limited to their reserved
   purpose.  If the data for a URI component would conflict with the
   reserved purpose, then the conflicting data must be escaped before
   forming the URI.

   reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" |
                    "$" | ","

(2.4.3)
   The angle-bracket "<" and ">" and double-quote (") characters are
   excluded because they are often used as the delimiters around URI in
   text documents and protocol fields.  The character "#" is excluded
   because it is used to delimit a URI from a fragment identifier in URI
   references (Section 4). The percent character "%" is excluded because
   it is used for the encoding of escaped characters.

   delims      = "<" | ">" | "#" | "%" | <">

   Other characters are excluded because gateways and other transport
   agents are known to sometimes modify such characters, or they are
   used as delimiters.

   unwise      = "{" | "}" | "|" | "\" | "^" | "[" | "]" | "`"

E.g. (the following change is placed in the public domain):

diff --git a/ctype.c b/ctype.c
index 56bdffa..01724d1 100644
--- a/ctype.c
+++ b/ctype.c
@@ -8,16 +8,17 @@ #include "cache.h"
 #define SS GIT_SPACE
 #define AA GIT_ALPHA
 #define DD GIT_DIGIT
+#define MM GIT_URIMARK	/* Legal URI "marks", RFC2396 section 2.3, plus / */
 
 unsigned char sane_ctype[256] = {
 	 0,  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 */
+	SS, MM,  0,  0,  0,  0,  0, MM, MM, MM, MM,  0,  0, MM, MM, MM,		/* 32-15 */
 	DD, DD, DD, DD, DD, DD, DD, DD, DD, DD,  0,  0,  0,  0,  0,  0,		/* 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,  0,  0,  0,  0, MM,		/* 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 */
+	AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,  0,  0,  0, MM,  0,		/* 112-15 */
 	/* Nothing in the 128.. range */
 };
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 5d543d2..7b4feae 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -138,11 +138,14 @@ extern unsigned char sane_ctype[256];
 #define GIT_SPACE 0x01
 #define GIT_DIGIT 0x02
 #define GIT_ALPHA 0x04
+#define GIT_URIMARK 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)
+/* Git extension - is character okay in URI (rfc2396, section 2.3), plus / */
+#define isuri(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT | GIT_URIMARK)
 #define tolower(x) sane_case((unsigned char)(x), 0x20)
 #define toupper(x) sane_case((unsigned char)(x), 0)
 

[PATCH 0/7] Improve ANSI C99 compliance

[Florian Forster]

Hi,

as promised I've split up the changes into several smaller patches. The changes
are grouped by type of problem they fix, as requested. The patches should not
depend on each other, but I didn't actually test that.

Regards,
-octo

[PATCH 1/7] Remove ranges from switch statements.

[Florian Forster]

Though very nice and readable, the "case 'a'...'z':" construct is not ANSI C99
compliant. This patch unfolds the range in `quote.c' and substitutes the
switch-statement with an if-statement in `http-fetch.c' and `http-push.c'.

Signed-off-by: Florian Forster <octo@verplant.org>


---

 http-fetch.c |   13 +++++++------
 http-push.c  |   13 +++++++------
 quote.c      |    9 ++++++++-
 3 files changed, 22 insertions(+), 13 deletions(-)

d90149c5b4e91938329120bdde609e5f6d9b03e8
diff --git a/http-fetch.c b/http-fetch.c
index da1a7f5..3a2cb5e 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1136,13 +1136,14 @@ int fetch(unsigned char *sha1)
 
 static inline int needs_quote(int ch)
 {
-	switch (ch) {
-	case '/': case '-': case '.':
-	case 'A'...'Z':	case 'a'...'z':	case '0'...'9':
+	if (((ch >= 'A') && (ch <= 'Z'))
+			|| ((ch >= 'a') && (ch <= 'z'))
+			|| ((ch >= '0') && (ch <= '9'))
+			|| (ch == '/')
+			|| (ch == '-')
+			|| (ch == '.'))
 		return 0;
-	default:
-		return 1;
-	}
+	return 1;
 }
 
 static inline int hex(int v)
diff --git a/http-push.c b/http-push.c
index 2d9441e..364ab76 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1077,13 +1077,14 @@ static int fetch_indices(void)
 
 static inline int needs_quote(int ch)
 {
-	switch (ch) {
-	case '/': case '-': case '.':
-	case 'A'...'Z':	case 'a'...'z':	case '0'...'9':
+	if (((ch >= 'A') && (ch <= 'Z'))
+			|| ((ch >= 'a') && (ch <= 'z'))
+			|| ((ch >= '0') && (ch <= '9'))
+			|| (ch == '/')
+			|| (ch == '-')
+			|| (ch == '.'))
 		return 0;
-	default:
-		return 1;
-	}
+	return 1;
 }
 
 static inline int hex(int v)
diff --git a/quote.c b/quote.c
index 06792d4..dcc2326 100644
--- a/quote.c
+++ b/quote.c
@@ -206,7 +206,14 @@ #define EMIT(c) (outp ? (*outp++ = (c)) 
 				case '\\': case '"':
 					break; /* verbatim */
 
-				case '0'...'7':
+				case '0':
+				case '1':
+				case '2':
+				case '3':
+				case '4':
+				case '5':
+				case '6':
+				case '7':
 					/* octal */
 					ac = ((ch - '0') << 6);
 					if ((ch = *sp++) < '0' || '7' < ch)
-- 
1.3.3

Re: [PATCH 1/7] Remove ranges from switch statements.

[Junio C Hamano]

Florian Forster <octo@verplant.org> writes:

> -	switch (ch) {
> -	case '/': case '-': case '.':
> -	case 'A'...'Z':	case 'a'...'z':	case '0'...'9':
> +	if (((ch >= 'A') && (ch <= 'Z'))
> +			|| ((ch >= 'a') && (ch <= 'z'))
> +			|| ((ch >= '0') && (ch <= '9'))
> + ...

Sorry for bringing up an old topic again, but wouldn't people
agree that this is easier to read if it were written this way ;-)?
 
	if (	   (('A' <= ch) && (ch <= 'Z'))
        	|| (('a' <= ch) && (ch <= 'z'))
		|| (('0' <= ch) && (ch <= '9'))
	...

Re: [PATCH 1/7] Remove ranges from switch statements.

[Timo Hirvonen]

Junio C Hamano <junkio@cox.net> wrote:

> Sorry for bringing up an old topic again, but wouldn't people
> agree that this is easier to read if it were written this way ;-)?
>  
> 	if (	   (('A' <= ch) && (ch <= 'Z'))
>         	|| (('a' <= ch) && (ch <= 'z'))
> 		|| (('0' <= ch) && (ch <= '9'))
> 	...

Yes, but isalnum(ch) even better ;)

-- 
http://onion.dynserv.net/~timo/