Re: Suggested patch: move dfa internals into dfa.c

[Jim Meyering]

Aharon Robbins wrote:
> Hi. The code
>
>       enum token_enum;
>       typedef token_enum token;
>
> isn't acceptable to at least two otherwise more-or-less C89 compilers.
>
> So, I went whole-hog and moved all the dfa internals into dfa.c. This
> fixes the long-standing wish in dfa.h about not exposing internals.
>
> Grep passes its test suite with this change, at least on my creaking-along
> Fedora 7 laptop. :-)
>
> Attached is a patch formatted per the HACKING file's instructions. It's
> my *very first* use of git, so be kind.
>
> Arnold
> --------------------------------------
>>From bc44560f893d7c86308479eb8070a4987360976e Mon Sep 17 00:00:00 2001
> From: Arnold D. Robbins <address@hidden>
> Date: Wed, 7 Apr 2010 21:42:39 +0300
> Subject: [PATCH] dfa.h: Moved essentially all the dfa internals out of dfa.h. 
> New routines added as needed.
>  dfa.c: The dfa internals are now totally local to this file. A few new 
> routines added for access to features.
>  dfasearch.c: Modified for changes in interface.

Thanks!
That looks like a fine change.

There were a few nits:
  - the fact that two #define'd macros were exposed as "unused"
      cause compilation failure when you use ./configure's
      --enable-gcc-warnings option.
  - lack of "void" between "()" caused another warning/failure.

I've adjusted your log message to look like this:

    dfa: move internals from dfa.h to dfa.c

    * src/dfa.h: Move internals into dfa.c
    * src/dfa.c: The dfa internals are now totally local to this file.
    (dfaalloc, dfamusts, dfabroken): New functions to access features.
    * src/dfasearch.c (dfa): Change this global variable from struct to pointer.
    Adapt to that change, and use new functions, dfamusts and dfaalloc.

I've squashed the following patch into yours and expect to push
the result as soon as I've run a few tests.

As I write this, I see that dfabroken should take a const pointer.
Will adjust that, too.

diff --git a/src/dfa.c b/src/dfa.c
index 9f6f165..fc73c42 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -70,7 +70,7 @@

 /* Number of bits in an unsigned char. */
 #ifndef CHARBITS
-#define CHARBITS 8
+# define CHARBITS 8
 #endif

 /* First integer value that is greater than any character code. */
@@ -78,7 +78,7 @@

 /* INTBITS need not be exact, just a lower bound. */
 #ifndef INTBITS
-#define INTBITS (CHARBITS * sizeof (int))
+# define INTBITS (CHARBITS * sizeof (int))
 #endif

 /* Number of ints required to hold a bit for every character. */
@@ -91,10 +91,6 @@
 /* Sets of unsigned characters are stored as bit vectors in arrays of ints. */
 typedef int charclass[CHARCLASS_INTS];

-/* Sets are stored in an array in the compiled dfa; the index of the
-   array corresponding to a given set token is given by SET_INDEX(t). */
-#define SET_INDEX(t) ((t) - CSET)
-
 /* Sometimes characters can only be matched depending on the surrounding
    context.  Such context decisions depend on what the previous character
    was, and the value of the current (lookahead) character.  Context
@@ -413,12 +409,6 @@ struct dfa
   SUCCEEDS_IN_CONTEXT((dfa).states[state].constraint,             \
                       prevn, currn, prevl, currl)

-/* FIRST_MATCHING_REGEXP returns the index number of the first of parallel
-   regexps that a given state could accept.  Parallel regexps are numbered
-   starting at 1. */
-#define FIRST_MATCHING_REGEXP(state, dfa) (-(dfa).states[state].first_end)
-
-
 static void dfamust (struct dfa *dfa);
 static void regexp (int toplevel);

@@ -3934,7 +3924,7 @@ dfamust (struct dfa *d)
 }

 struct dfa *
-dfaalloc()
+dfaalloc (void)
 {
   return xmalloc (sizeof(struct dfa));
 }
@@ -3946,7 +3936,7 @@ dfamusts (struct dfa *d)
 }

 #ifdef GAWK
-int dfabroken (struct dfa * d)
+int dfabroken (struct dfa *d)
 {
   return d->broken;
 }
diff --git a/src/dfa.h b/src/dfa.h
index 0721cc0..018629a 100644
--- a/src/dfa.h
+++ b/src/dfa.h
@@ -35,7 +35,7 @@ struct dfa;
 /* Allocate a struct dfa.  The struct dfa is completely opaque.
    The returned pointer should be passed directly to free() after
    calling dfafree() on it. */
-extern struct dfa *dfaalloc ();
+extern struct dfa *dfaalloc (void);

 /* Return the dfamusts associated with a dfa. */
 extern struct dfamust *dfamusts (struct dfa *);