Re: src/kwset.c (kwsincr): Replace arbitrary array size by proper value

[Charles Levert]

* On Monday 2005-07-04 at 22:44:43 -0700, Paul Eggert wrote:
> Charles Levert <address@hidden> writes:
> 
> > I have checked that this will hold for CHAR_BIT
> > up to 1023.

I've put this in the ChangeLog.


> > Custom tools used for checking
> > this available upon request (part analytical,
> > part experimental).
> 
> Can you please put a comment to this effect in the source code, where
> it's less likely to get lost?  It would help to document as much as
> you can, as long as it's concise.

See below.


> Please change the name from DEPTH_SIZE to DEPTH_BOUND (since it's
> an upper bound),

I wanted to avoid calling it DEPTH_MAX,
because the values (array indexes) go from 0
to DEPTH_SIZE - 1.

I chose DEPTH_SIZE because this is unambiguous
in the context of the C language and its 0-based
arrays.  The common idiom in C is

   for (i = 0; i < SIZE; i++)

I would argue that DEPTH_BOUND carries an
ambiguity between DEPTH_SIZE and DEPTH_MAX
because it can be interpreted as pertaining to
array indexes.  (I.e., is it defined as SIZE
or SIZE - 1?  Is it the last valid value or the
first invalid one?)

There's also that many occurences of the word
BOUND in cpp macro names is as the opposite
of UNBOUND (e.g., with sockets), for enum-like
constants.

Here, the SIZE just happens to be defined to a
mathematical upper bound of it, as opposed to
an always-exact value.

As for LENGTH, I prefer to reserve it for
'\0'-terminated strings where LENGTH is
SIZE - 1.


> and please avoid the #else.  Also, please indent
> the preprocessing directive, and please use a clearer diagnostic.
> Finally, please use the GNU coding style for spacing within expressions.
> 
> Something like this, perhaps:
> 
> 
> /* Upper bound for depth of trie.  This bound is valid for CHAR_BIT >= 4 and
>    exact for CHAR_BIT in { 4..11, 13, 15, 17, 19 }.  For details, please
>    see grep-internals.texi.  */

We don't have a "grep-internals.texi".  Is it
worth it to create one for only this (at this
time)?

I am ready to document the background behind
the formula.

My only fear is that there likely already is a
published paper proving just this, probably in
a much more elegant way, and that I don't know
about it.  Does someone here know?  We could
just reference to it, stating that it contains
this precise development.


> #if CHAR_BIT < 4
> # error "configuration error: impossible CHAR_BIT"
> #endif

Others have suggested not having #error
altogether because it would be a first in GNU
grep, so I got rid of it.  Is #error portable
all the way back to older non-standard compilers?


> #define DEPTH_BOUND (CHAR_BIT + CHAR_BIT / 2)

Ok for the spacing.  It was an oversight on my
part, because I usually like this too.