source@mandoc.bsd.lv
 help / color / mirror / Atom feed
* mdocml: Instead of a series of hard-coded strcmps, make argv_a2arg()
@ 2011-03-16 17:55 kristaps
  2011-03-16 18:00 ` Joerg Sonnenberger
  0 siblings, 1 reply; 2+ messages in thread
From: kristaps @ 2011-03-16 17:55 UTC (permalink / raw)
  To: source

Log Message:
-----------
Instead of a series of hard-coded strcmps, make argv_a2arg() iterate
over an array of possible argument types, hooking in to mdoc_argnames[].
This knocks off bytes for the duplicated strings and cleans up code
readability.

Modified Files:
--------------
    mdocml:
        mdoc_argv.c

Revision Data
-------------
Index: mdoc_argv.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_argv.c,v
retrieving revision 1.62
retrieving revision 1.63
diff -Lmdoc_argv.c -Lmdoc_argv.c -u -p -r1.62 -r1.63
--- mdoc_argv.c
+++ mdoc_argv.c
@@ -583,102 +583,71 @@ args(struct mdoc *m, int line, int *pos,
 	return(ARGS_WORD);
 }
 
-
+/*
+ * Match up an argument string (e.g., `-foo bar' having "foo") with the
+ * correrct identifier.  It must apply to the given macro.  If none was
+ * found (including bad matches), return MDOC_ARG_MAX.
+ */
 static enum mdocargt
 argv_a2arg(enum mdoct tok, const char *p)
 {
+	enum mdocargt	 args[MDOC_ARG_MAX];
+	int		 i, len;
 
-	/*
-	 * Parse an argument identifier from its text.  XXX - this
-	 * should really be table-driven to clarify the code.
-	 *
-	 * If you add an argument to the list, make sure that you
-	 * register it here with its one or more macros!
-	 */
+	len = 0;
 
 	switch (tok) {
 	case (MDOC_An):
-		if (0 == strcmp(p, "split"))
-			return(MDOC_Split);
-		else if (0 == strcmp(p, "nosplit"))
-			return(MDOC_Nosplit);
+		args[len++] = MDOC_Split;
+		args[len++] = MDOC_Nosplit;
 		break;
-
 	case (MDOC_Bd):
-		if (0 == strcmp(p, "ragged"))
-			return(MDOC_Ragged);
-		else if (0 == strcmp(p, "unfilled"))
-			return(MDOC_Unfilled);
-		else if (0 == strcmp(p, "filled"))
-			return(MDOC_Filled);
-		else if (0 == strcmp(p, "literal"))
-			return(MDOC_Literal);
-		else if (0 == strcmp(p, "file"))
-			return(MDOC_File);
-		else if (0 == strcmp(p, "offset"))
-			return(MDOC_Offset);
-		else if (0 == strcmp(p, "compact"))
-			return(MDOC_Compact);
-		else if (0 == strcmp(p, "centered"))
-			return(MDOC_Centred);
+		args[len++] = MDOC_Ragged;
+		args[len++] = MDOC_Unfilled;
+		args[len++] = MDOC_Filled;
+		args[len++] = MDOC_Literal;
+		args[len++] = MDOC_File;
+		args[len++] = MDOC_Offset;
+		args[len++] = MDOC_Compact;
+		args[len++] = MDOC_Centred;
 		break;
-
 	case (MDOC_Bf):
-		if (0 == strcmp(p, "emphasis"))
-			return(MDOC_Emphasis);
-		else if (0 == strcmp(p, "literal"))
-			return(MDOC_Literal);
-		else if (0 == strcmp(p, "symbolic"))
-			return(MDOC_Symbolic);
+		args[len++] = MDOC_Emphasis;
+		args[len++] = MDOC_Literal;
+		args[len++] = MDOC_Symbolic;
 		break;
-
 	case (MDOC_Bk):
-		if (0 == strcmp(p, "words"))
-			return(MDOC_Words);
+		args[len++] = MDOC_Words;
 		break;
-
 	case (MDOC_Bl):
-		if (0 == strcmp(p, "bullet"))
-			return(MDOC_Bullet);
-		else if (0 == strcmp(p, "dash"))
-			return(MDOC_Dash);
-		else if (0 == strcmp(p, "hyphen"))
-			return(MDOC_Hyphen);
-		else if (0 == strcmp(p, "item"))
-			return(MDOC_Item);
-		else if (0 == strcmp(p, "enum"))
-			return(MDOC_Enum);
-		else if (0 == strcmp(p, "tag"))
-			return(MDOC_Tag);
-		else if (0 == strcmp(p, "diag"))
-			return(MDOC_Diag);
-		else if (0 == strcmp(p, "hang"))
-			return(MDOC_Hang);
-		else if (0 == strcmp(p, "ohang"))
-			return(MDOC_Ohang);
-		else if (0 == strcmp(p, "inset"))
-			return(MDOC_Inset);
-		else if (0 == strcmp(p, "column"))
-			return(MDOC_Column);
-		else if (0 == strcmp(p, "width"))
-			return(MDOC_Width);
-		else if (0 == strcmp(p, "offset"))
-			return(MDOC_Offset);
-		else if (0 == strcmp(p, "compact"))
-			return(MDOC_Compact);
-		else if (0 == strcmp(p, "nested"))
-			return(MDOC_Nested);
+		args[len++] = MDOC_Bullet;
+		args[len++] = MDOC_Dash;
+		args[len++] = MDOC_Hyphen;
+		args[len++] = MDOC_Item;
+		args[len++] = MDOC_Enum;
+		args[len++] = MDOC_Tag;
+		args[len++] = MDOC_Diag;
+		args[len++] = MDOC_Hang;
+		args[len++] = MDOC_Ohang;
+		args[len++] = MDOC_Inset;
+		args[len++] = MDOC_Column;
+		args[len++] = MDOC_Width;
+		args[len++] = MDOC_Offset;
+		args[len++] = MDOC_Compact;
+		args[len++] = MDOC_Nested;
 		break;
-	
 	case (MDOC_Rv):
 		/* FALLTHROUGH */
 	case (MDOC_Ex):
-		if (0 == strcmp(p, "std"))
-			return(MDOC_Std);
+		args[len++] = MDOC_Std;
 		break;
 	default:
 		break;
 	}
+
+	for (i = 0; i < len; i++)
+		if (0 == strcmp(p, mdoc_argnames[args[i]]))
+			return(args[i]);
 
 	return(MDOC_ARG_MAX);
 }
--
 To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: mdocml: Instead of a series of hard-coded strcmps, make argv_a2arg()
  2011-03-16 17:55 mdocml: Instead of a series of hard-coded strcmps, make argv_a2arg() kristaps
@ 2011-03-16 18:00 ` Joerg Sonnenberger
  0 siblings, 0 replies; 2+ messages in thread
From: Joerg Sonnenberger @ 2011-03-16 18:00 UTC (permalink / raw)
  To: source

On Wed, Mar 16, 2011 at 01:55:39PM -0400, kristaps@mdocml.bsd.lv wrote:
> +/*
> + * Match up an argument string (e.g., `-foo bar' having "foo") with the
> + * correrct identifier.  It must apply to the given macro.  If none was

           ^ typo

> + * found (including bad matches), return MDOC_ARG_MAX.
> + */
>  static enum mdocargt
>  argv_a2arg(enum mdoct tok, const char *p)
>  {
> +	enum mdocargt	 args[MDOC_ARG_MAX];
> +	int		 i, len;
>  
> -	/*
> -	 * Parse an argument identifier from its text.  XXX - this
> -	 * should really be table-driven to clarify the code.
> -	 *
> -	 * If you add an argument to the list, make sure that you
> -	 * register it here with its one or more macros!
> -	 */
> +	len = 0;
>  
>  	switch (tok) {
>  	case (MDOC_An):
> -		if (0 == strcmp(p, "split"))
> -			return(MDOC_Split);
> -		else if (0 == strcmp(p, "nosplit"))
> -			return(MDOC_Nosplit);
> +		args[len++] = MDOC_Split;
> +		args[len++] = MDOC_Nosplit;
>  		break;
> -
>  	case (MDOC_Bd):
> -		if (0 == strcmp(p, "ragged"))
> -			return(MDOC_Ragged);
> -		else if (0 == strcmp(p, "unfilled"))
> -			return(MDOC_Unfilled);
> -		else if (0 == strcmp(p, "filled"))
> -			return(MDOC_Filled);
> -		else if (0 == strcmp(p, "literal"))
> -			return(MDOC_Literal);
> -		else if (0 == strcmp(p, "file"))
> -			return(MDOC_File);
> -		else if (0 == strcmp(p, "offset"))
> -			return(MDOC_Offset);
> -		else if (0 == strcmp(p, "compact"))
> -			return(MDOC_Compact);
> -		else if (0 == strcmp(p, "centered"))
> -			return(MDOC_Centred);
> +		args[len++] = MDOC_Ragged;
> +		args[len++] = MDOC_Unfilled;
> +		args[len++] = MDOC_Filled;
> +		args[len++] = MDOC_Literal;
> +		args[len++] = MDOC_File;
> +		args[len++] = MDOC_Offset;
> +		args[len++] = MDOC_Compact;
> +		args[len++] = MDOC_Centred;
>  		break;
> -
>  	case (MDOC_Bf):
> -		if (0 == strcmp(p, "emphasis"))
> -			return(MDOC_Emphasis);
> -		else if (0 == strcmp(p, "literal"))
> -			return(MDOC_Literal);
> -		else if (0 == strcmp(p, "symbolic"))
> -			return(MDOC_Symbolic);
> +		args[len++] = MDOC_Emphasis;
> +		args[len++] = MDOC_Literal;
> +		args[len++] = MDOC_Symbolic;
>  		break;
> -
>  	case (MDOC_Bk):
> -		if (0 == strcmp(p, "words"))
> -			return(MDOC_Words);
> +		args[len++] = MDOC_Words;
>  		break;
> -
>  	case (MDOC_Bl):
> -		if (0 == strcmp(p, "bullet"))
> -			return(MDOC_Bullet);
> -		else if (0 == strcmp(p, "dash"))
> -			return(MDOC_Dash);
> -		else if (0 == strcmp(p, "hyphen"))
> -			return(MDOC_Hyphen);
> -		else if (0 == strcmp(p, "item"))
> -			return(MDOC_Item);
> -		else if (0 == strcmp(p, "enum"))
> -			return(MDOC_Enum);
> -		else if (0 == strcmp(p, "tag"))
> -			return(MDOC_Tag);
> -		else if (0 == strcmp(p, "diag"))
> -			return(MDOC_Diag);
> -		else if (0 == strcmp(p, "hang"))
> -			return(MDOC_Hang);
> -		else if (0 == strcmp(p, "ohang"))
> -			return(MDOC_Ohang);
> -		else if (0 == strcmp(p, "inset"))
> -			return(MDOC_Inset);
> -		else if (0 == strcmp(p, "column"))
> -			return(MDOC_Column);
> -		else if (0 == strcmp(p, "width"))
> -			return(MDOC_Width);
> -		else if (0 == strcmp(p, "offset"))
> -			return(MDOC_Offset);
> -		else if (0 == strcmp(p, "compact"))
> -			return(MDOC_Compact);
> -		else if (0 == strcmp(p, "nested"))
> -			return(MDOC_Nested);
> +		args[len++] = MDOC_Bullet;
> +		args[len++] = MDOC_Dash;
> +		args[len++] = MDOC_Hyphen;
> +		args[len++] = MDOC_Item;
> +		args[len++] = MDOC_Enum;
> +		args[len++] = MDOC_Tag;
> +		args[len++] = MDOC_Diag;
> +		args[len++] = MDOC_Hang;
> +		args[len++] = MDOC_Ohang;
> +		args[len++] = MDOC_Inset;
> +		args[len++] = MDOC_Column;
> +		args[len++] = MDOC_Width;
> +		args[len++] = MDOC_Offset;
> +		args[len++] = MDOC_Compact;
> +		args[len++] = MDOC_Nested;
>  		break;

Why are the tables not static const data? That's IMO more readable too.

> -	
>  	case (MDOC_Rv):
>  		/* FALLTHROUGH */
>  	case (MDOC_Ex):
> -		if (0 == strcmp(p, "std"))
> -			return(MDOC_Std);
> +		args[len++] = MDOC_Std;
>  		break;
>  	default:
>  		break;
>  	}
> +
> +	for (i = 0; i < len; i++)
> +		if (0 == strcmp(p, mdoc_argnames[args[i]]))
> +			return(args[i]);
>  
>  	return(MDOC_ARG_MAX);
>  }

What about storing the size of the strings too and matching on that
first? Might be helpful or not.

Joerg
--
 To unsubscribe send an email to source+unsubscribe@mdocml.bsd.lv

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-03-16 18:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-16 17:55 mdocml: Instead of a series of hard-coded strcmps, make argv_a2arg() kristaps
2011-03-16 18:00 ` Joerg Sonnenberger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).