Stage 2: use accessors, not direct reference, into argv

5 files changed, 125 insertions, 47 deletions

diff --git a/src/builtin.c b/src/builtin.c
index 14e70346..5be5cc44 100644
--- a/src/builtin.c
+++ b/src/builtin.c
@@ -30,9 +30,9 @@
 # include <sys/wait.h>
 #endif
 
-#define ARG(i)								\
-  ((i) == 0 ? argv->argv0						\
-   : argv->argc > (i) ? TOKEN_DATA_TEXT (argv->array[(i) - 1]) : "")
+/* Grab the text at argv index I.  Assumes a macro_argument *argv is
+   in scope.  */
+#define ARG(i) arg_text (argv, i)
 
 /* Initialization of builtin and predefined macros.  The table
    "builtin_tab" is both used for initialization, and by the "builtin"
@@ -598,20 +598,22 @@ shipout_int (struct obstack *obs, int val)
   obstack_grow (obs, s, strlen (s));
 }
 
-/*----------------------------------------------------------------------.
-| Print ARGC arguments from the table ARGV to obstack OBS, separated by |
-| SEP, and quoted by the current quotes, if QUOTED is true.	        |
-`----------------------------------------------------------------------*/
+/*------------------------------------------------------------------.
+| Print arguments from the table ARGV to obstack OBS, starting with |
+| START, separated by SEP, and quoted by the current quotes if	    |
+| QUOTED is true.						    |
+`------------------------------------------------------------------*/
 
 static void
 dump_args (struct obstack *obs, int start, macro_arguments *argv,
 	   const char *sep, bool quoted)
 {
-  int i;
+  unsigned int i;
   bool dump_sep = false;
   size_t len = strlen (sep);
+  unsigned int argc = arg_argc (argv);
 
-  for (i = start; i < argv->argc; i++)
+  for (i = start; i < argc; i++)
     {
       if (dump_sep)
 	obstack_grow (obs, sep, len);
@@ -655,7 +657,7 @@ define_macro (int argc, macro_arguments *argv, symbol_lookup mode)
   if (bad_argc (me, argc, 1, 2))
     return;
 
-  if (TOKEN_DATA_TYPE (argv->array[0]) != TOKEN_TEXT)
+  if (arg_type (argv, 1) != TOKEN_TEXT)
     {
       m4_warn (0, me, _("invalid macro name ignored"));
       return;
@@ -667,14 +669,14 @@ define_macro (int argc, macro_arguments *argv, symbol_lookup mode)
       return;
     }
 
-  switch (TOKEN_DATA_TYPE (argv->array[1]))
+  switch (arg_type (argv, 2))
     {
     case TOKEN_TEXT:
       define_user_macro (ARG (1), ARG (2), mode);
       break;
 
     case TOKEN_FUNC:
-      bp = find_builtin_by_addr (TOKEN_DATA_FUNC (argv->array[1]));
+      bp = find_builtin_by_addr (arg_func (argv, 2));
       if (bp == NULL)
 	return;
       else
@@ -915,7 +917,7 @@ m4_builtin (struct obstack *obs, int argc, macro_arguments *argv)
 
   if (bad_argc (me, argc, 1, -1))
     return;
-  if (TOKEN_DATA_TYPE (argv->array[0]) != TOKEN_TEXT)
+  if (arg_type (argv, 1) != TOKEN_TEXT)
     {
       m4_warn (0, me, _("invalid macro name ignored"));
       return;
@@ -929,6 +931,8 @@ m4_builtin (struct obstack *obs, int argc, macro_arguments *argv)
     {
       int i;
       // TODO make use of $@ reference, instead of copying argv
+      // TODO make accessor in macro.c that performs this
+      // construction, so that argv can be opaque type.
       macro_arguments *new_argv = xmalloc (offsetof (macro_arguments, array)
 					   + ((argc - 2)
 					      * sizeof (token_data *)));
@@ -940,7 +944,7 @@ m4_builtin (struct obstack *obs, int argc, macro_arguments *argv)
 	      (argc - 2) * sizeof (token_data *));
       if (!bp->groks_macro_args)
 	for (i = 2; i < argc; i++)
-	  if (TOKEN_DATA_TYPE (new_argv->array[i - 2]) != TOKEN_TEXT)
+	  if (arg_type (argv, i) != TOKEN_TEXT)
 	    {
 	      TOKEN_DATA_TYPE (new_argv->array[i - 2]) = TOKEN_TEXT;
 	      TOKEN_DATA_TEXT (new_argv->array[i - 2]) = (char *) "";
@@ -966,7 +970,7 @@ m4_indir (struct obstack *obs, int argc, macro_arguments *argv)
 
   if (bad_argc (me, argc, 1, -1))
     return;
-  if (TOKEN_DATA_TYPE (argv->array[0]) != TOKEN_TEXT)
+  if (arg_type (argv, 1) != TOKEN_TEXT)
     {
       m4_warn (0, me, _("invalid macro name ignored"));
       return;
@@ -980,6 +984,8 @@ m4_indir (struct obstack *obs, int argc, macro_arguments *argv)
     {
       int i;
       // TODO make use of $@ reference, instead of copying argv
+      // TODO make accessor in macro.c that performs this
+      // construction, so that argv can be opaque type.
       macro_arguments *new_argv = xmalloc (offsetof (macro_arguments, array)
 					   + ((argc - 2)
 					      * sizeof (token_data *)));
@@ -991,7 +997,7 @@ m4_indir (struct obstack *obs, int argc, macro_arguments *argv)
 	      (argc - 2) * sizeof (token_data *));
       if (!SYMBOL_MACRO_ARGS (s))
 	for (i = 2; i < argc; i++)
-	  if (TOKEN_DATA_TYPE (new_argv->array[i - 2]) != TOKEN_TEXT)
+	  if (arg_type (argv, i) != TOKEN_TEXT)
 	    {
 	      TOKEN_DATA_TYPE (new_argv->array[i - 2]) = TOKEN_TEXT;
 	      TOKEN_DATA_TEXT (new_argv->array[i - 2]) = (char *) "";
diff --git a/src/debug.c b/src/debug.c
index e5bd2808..c4f701d9 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -379,14 +379,14 @@ trace_pre (const char *name, int id, int argc, macro_arguments *argv)
 	  if (i != 1)
 	    trace_format (", ");
 
-	  switch (TOKEN_DATA_TYPE (argv->array[i - 1]))
+	  switch (arg_type (argv, i))
 	    {
 	    case TOKEN_TEXT:
-	      trace_format ("%l%S%r", TOKEN_DATA_TEXT (argv->array[i - 1]));
+	      trace_format ("%l%S%r", arg_text (argv, i));
 	      break;
 
 	    case TOKEN_FUNC:
-	      bp = find_builtin_by_addr (TOKEN_DATA_FUNC (argv->array[i - 1]));
+	      bp = find_builtin_by_addr (arg_func (argv, i));
 	      if (bp == NULL)
 		{
 		  assert (!"trace_pre");
diff --git a/src/format.c b/src/format.c
index 4c2b60a9..7fc8fb16 100644
--- a/src/format.c
+++ b/src/format.c
@@ -28,16 +28,16 @@
    same size; likewise for long and unsigned long.  */
 
 #define ARG_INT(i, argc, argv)						\
-	((i == argc) ? 0 : atoi (TOKEN_DATA_TEXT (argv->array[i++ - 1])))
+  ((i == argc) ? 0 : atoi (arg_text (argv, i++)))
 
 #define ARG_LONG(i, argc, argv)						\
-	((i == argc) ? 0L : atol (TOKEN_DATA_TEXT (argv->array[i++ - 1])))
+  ((i == argc) ? 0L : atol (arg_text (argv, i++)))
 
 #define ARG_STR(i, argc, argv)						\
-	((i == argc) ? "" : TOKEN_DATA_TEXT (argv->array[i++ - 1]))
+  ((i == argc) ? "" : arg_text (argv, i++))
 
 #define ARG_DOUBLE(i, argc, argv)					\
-	((i == argc) ? 0.0 : atof (TOKEN_DATA_TEXT (argv->array[i++ - 1])))
+  ((i == argc) ? 0.0 : atof (arg_text (argv, i++)))
 
 
 /*------------------------------------------------------------------.
diff --git a/src/m4.h b/src/m4.h
index 94276e9f..c9cb843b 100644
--- a/src/m4.h
+++ b/src/m4.h
@@ -178,7 +178,7 @@ extern FILE *debug;
 /* default flags -- equiv: aeq */
 #define DEBUG_TRACE_DEFAULT 7
 
-#define DEBUG_PRINT1(Fmt, Arg1) \
+#define DEBUG_PRINT1(Fmt, Arg1)                                 \
   do								\
     {								\
       if (debug != NULL)					\
@@ -186,7 +186,7 @@ extern FILE *debug;
     }								\
   while (0)
 
-#define DEBUG_PRINT3(Fmt, Arg1, Arg2, Arg3) \
+#define DEBUG_PRINT3(Fmt, Arg1, Arg2, Arg3)                     \
   do								\
     {								\
       if (debug != NULL)					\
@@ -194,7 +194,7 @@ extern FILE *debug;
     }								\
   while (0)
 
-#define DEBUG_MESSAGE(Fmt) \
+#define DEBUG_MESSAGE(Fmt)                                      \
   do								\
     {								\
       if (debug != NULL)					\
@@ -206,7 +206,7 @@ extern FILE *debug;
     }								\
   while (0)
 
-#define DEBUG_MESSAGE1(Fmt, Arg1) \
+#define DEBUG_MESSAGE1(Fmt, Arg1)                               \
   do								\
     {								\
       if (debug != NULL)					\
@@ -218,7 +218,7 @@ extern FILE *debug;
     }								\
   while (0)
 
-#define DEBUG_MESSAGE2(Fmt, Arg1, Arg2) \
+#define DEBUG_MESSAGE2(Fmt, Arg1, Arg2)                         \
   do								\
     {								\
       if (debug != NULL)					\
@@ -244,37 +244,39 @@ void trace_post (const char *, int, int, macro_arguments *, const char *);
 
 typedef struct token_chain token_chain;
 
-/* Various different token types.  */
+/* Various different token types.  Avoid overlap with token_data_type,
+   since the shared prefix of the enumerators is a bit confusing.  */
 enum token_type
 {
-  TOKEN_EOF,			/* end of file */
-  TOKEN_STRING,			/* a quoted string or comment */
-  TOKEN_WORD,			/* an identifier */
-  TOKEN_OPEN,			/* ( */
-  TOKEN_COMMA,			/* , */
-  TOKEN_CLOSE,			/* ) */
-  TOKEN_SIMPLE,			/* any other single character */
-  TOKEN_MACDEF			/* a macro's definition (see "defn") */
+  TOKEN_EOF = 4,/* End of file, TOKEN_VOID.  */
+  TOKEN_STRING,	/* Quoted string or comment, TOKEN_TEXT or TOKEN_COMP.  */
+  TOKEN_WORD,	/* An identifier, TOKEN_TEXT.  */
+  TOKEN_OPEN,	/* Active character `(', TOKEN_TEXT.  */
+  TOKEN_COMMA,	/* Active character `,', TOKEN_TEXT.  */
+  TOKEN_CLOSE,	/* Active character `)', TOKEN_TEXT.  */
+  TOKEN_SIMPLE,	/* Any other single character, TOKEN_TEXT.  */
+  TOKEN_MACDEF	/* A macro's definition (see "defn"), TOKEN_FUNC.  */
 };
 
 /* The data for a token, a macro argument, and a macro definition.  */
 enum token_data_type
 {
-  TOKEN_VOID, /* Token still being constructed, u is invalid.  */
-  TOKEN_TEXT, /* Straight text, u.u_t is valid.  */
-  TOKEN_FUNC, /* Builtin function definition, u.func is valid.  */
-  TOKEN_COMP  /* Composite argument, u.chain is valid.  */
+  TOKEN_VOID,	/* Token still being constructed, u is invalid.  */
+  TOKEN_TEXT,	/* Straight text, u.u_t is valid.  */
+  TOKEN_FUNC,	/* Builtin function definition, u.func is valid.  */
+  TOKEN_COMP	/* Composite argument, u.chain is valid.  */
 };
 
 /* Composite tokens are built of a linked list of chains.  */
 struct token_chain
 {
-  token_chain *next; /* Pointer to next link of chain.  */
-  char *str; /* NUL-terminated string if text, else NULL.  */
-  macro_arguments *argv; /* Reference to earlier $@.  */
-  unsigned int index; /* Index within argv to start reading from.  */
+  token_chain *next;	/* Pointer to next link of chain.  */
+  char *str;		/* NUL-terminated string if text, else NULL.  */
+  macro_arguments *argv;/* Reference to earlier $@.  */
+  unsigned int index;	/* Argument index within argv.  */
 };
 
+/* The content of a token or macro argument.  */
 struct token_data
 {
   enum token_data_type type;
@@ -297,6 +299,9 @@ struct token_data
   u;
 };
 
+// TODO - make this struct opaque, and move definition to macro.c
+/* Opaque structure describing all arguments to a macro, including the
+   macro name at index 0.  */
 struct macro_arguments
 {
   /* Number of arguments owned by this object, may be larger than
@@ -427,6 +432,13 @@ extern int expansion_level;
 
 void expand_input (void);
 void call_macro (symbol *, int, macro_arguments *, struct obstack *);
+
+unsigned int arg_argc (macro_arguments *);
+token_data_type arg_type (macro_arguments *, unsigned int);
+const char *arg_text (macro_arguments *, unsigned int);
+size_t arg_len (macro_arguments *, unsigned int);
+builtin_func *arg_func (macro_arguments *, unsigned int);
+
 
 /* File: builtin.c  --- builtins.  */
 
diff --git a/src/macro.c b/src/macro.c
index 7f566dc4..1a8e61c5 100644
--- a/src/macro.c
+++ b/src/macro.c
@@ -242,7 +242,7 @@ expand_argument (struct obstack *obs, token_data *argp, const char *caller)
 `-------------------------------------------------------------------------*/
 
 static macro_arguments *
-collect_arguments (symbol *sym, struct obstack *argptr, unsigned argv_base,
+collect_arguments (symbol *sym, struct obstack *argptr, unsigned int argv_base,
 		   struct obstack *arguments)
 {
   token_data td;
@@ -327,7 +327,7 @@ static void
 expand_macro (symbol *sym)
 {
   struct obstack arguments;	/* Alternate obstack if arg_stack is busy.  */
-  unsigned argv_base;		/* Size of argv_stack on entry.  */
+  unsigned int argv_base;	/* Size of argv_stack on entry.  */
   void *arg_start;		/* Start of arg_stack, else NULL if unsafe.  */
   macro_arguments *argv;
   int argc;
@@ -409,3 +409,63 @@ expand_macro (symbol *sym)
     obstack_free (&arguments, NULL);
   obstack_blank (&argv_stack, argv_base - obstack_object_size (&argv_stack));
 }
+
+
+/* Given ARGV, return how many arguments it refers to.  */
+unsigned int
+arg_argc (macro_arguments *argv)
+{
+  return argv->argc;
+}
+
+/* Given ARGV, return the type of argument INDEX.  Index 0 is always
+   text, and indices beyond argc are likewise treated as text.  */
+token_data_type
+arg_type (macro_arguments *argv, unsigned int index)
+{
+  if (index == 0 || index >= argv->argc)
+    return TOKEN_TEXT;
+  return TOKEN_DATA_TYPE (argv->array[index - 1]);
+}
+
+/* Given ARGV, return the text at argument INDEX, or NULL if the
+   argument is not text.  Index 0 is always text, and indices beyond
+   argc return the empty string.  */
+const char *
+arg_text (macro_arguments *argv, unsigned int index)
+{
+  if (index == 0)
+    return argv->argv0;
+  if (index >= argv->argc)
+    return "";
+  if (TOKEN_DATA_TYPE (argv->array[index - 1]) != TOKEN_TEXT)
+    return NULL;
+  return TOKEN_DATA_TEXT (argv->array[index - 1]);
+}
+
+/* Given ARGV, return the length of argument INDEX, or SIZE_MAX if the
+   argument is not text.  Indices beyond argc return 0.  */
+size_t
+arg_len (macro_arguments *argv, unsigned int index)
+{
+  // TODO - update macro_arguments to cache this...
+  if (index == 0)
+    return strlen (argv->argv0);
+  if (index >= argv->argc)
+    return 0;
+  if (TOKEN_DATA_TYPE (argv->array[index - 1]) != TOKEN_TEXT)
+    return SIZE_MAX;
+  return strlen (TOKEN_DATA_TEXT (argv->array[index - 1]));
+}
+
+/* Given ARGV, return the builtin function referenced by argument
+   INDEX, or NULL if it is not a builtin.  Index 0, and indices beyond
+   argc, return NULL.  */
+builtin_func *
+arg_func (macro_arguments *argv, unsigned int index)
+{
+  if (index == 0 || index >= argv->argc
+      || TOKEN_DATA_TYPE (argv->array[index - 1]) != TOKEN_FUNC)
+    return NULL;
+  return TOKEN_DATA_FUNC (argv->array[index - 1]);
+}