[PATCH 3/6] Clean: parse_token_word(): relax memory requirements (off by one)

[Michael Witten]

This doesn't really fix a bug; it just relaxes the required `room' that is
necessary in the allocated buffer.

>From general.h:164:

  #define RESIZE_MALLOCED_BUFFER(str, cind, room, csize, sincr) \
    do { \
      if ((cind) + (room) >= csize) \
        { \
          while ((cind) + (room) >= csize) \
            csize += (sincr); \
          str = xrealloc (str, csize); \
        } \
    } while (0)

Firstly, RESIZE_MALLOCED_BUFFER() is written to assume that `cindex'
is the largest index such that `str[index]' is live data. However, in
parse_token_word(), RESIZE_MALLOCED_BUFFER() is given `token_index' as
`cindex' when the required index is actually `token_index-1', thereby
inflating the measurement of used space by one.

An alternative interpretation is that the value of `token_index' is the
number of live characters already stored in the `token' array. Then the
macro's allocation condition:

    (cind) + (room) >= csize

Can be reinterpreted as:

    (# of live slots) + (# of slots to be made live) >= (# of slots in buffer)

or:

    `str' must have at least 1 unused slot after all live data
    has been slotted.

What is this 1 unused slot for? Well, in this case, it seems to
be for the null character '\0' placed there by strcpy() or
when the token is complete.

Thus, the code in question:

  RESIZE_MALLOCED_BUFFER (token, token_index, ttranslen + 2,
                          token_buffer_size,
                          TOKEN_DEFAULT_GROW_SIZE);

says:

    `token' must have at least 1 unused slot after all live data
    has been slotted.

However, it lists the number of live slots as:

  cind + room = token_index + ttranslen + 2

when the actual number of live slots is:

  (# of live slots) + (# of slots to be made live) = token_index + ttranslen

So, it would seem that there would be enough for the final '\0' character
if the requirement were reduced to:

  RESIZE_MALLOCED_BUFFER (token, token_index, ttranslen,
                          token_buffer_size,
                          TOKEN_DEFAULT_GROW_SIZE);

However, there's one more wrinkle: parse_token_word() has the
following code for handling CTLESC and CTLNUL:

    if (character == CTLESC || character == CTLNUL)
      token[token_index++] = CTLESC;

    /* ... minor stuff in between ... */

    token[token_index++] = character;

    RESIZE_MALLOCED_BUFFER (token, token_index, 1, token_buffer_size,
                            TOKEN_DEFAULT_GROW_SIZE);

As you can see, each CTLESC or CTLNUL encountered on the input is added to
the token only after a CTLESC has been artificially inserted first; this
means that there must actually be room not just for the '\0' added in the
last slot when the token is complete, but also for this extra CTLESC. That
is, parse_token_word()'s overall requirement is:

    `token' must have at least 2 unused slots while a CTLESC or CTLNUL
    may still be read, one for the extra CTLESC and one for the final
    '\0' character.

Thus, we need to inflate the `room' value by 1, from `ttranslen' to
`ttranslen + 1':

  RESIZE_MALLOCED_BUFFER (token, token_index, ttranslen + 1,
                          token_buffer_size,
                          TOKEN_DEFAULT_GROW_SIZE);

This patch achieves that final result by deflating the `room' value by 1,
from `ttranslen + 2' to `ttranslen + 1'.

Signed-off-by: Michael Witten <mfwitten@gmail.com>
---
 parse.y |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/parse.y b/parse.y
index 55874aa..38c4330 100644
--- a/parse.y
+++ b/parse.y
@@ -4524,7 +4524,7 @@ read_token_word (character)
                  ttrans = ttok;
                }
 
-             RESIZE_MALLOCED_BUFFER (token, token_index, ttranslen + 2,
+             RESIZE_MALLOCED_BUFFER (token, token_index, ttranslen + 1,
                                      token_buffer_size,
                                      TOKEN_DEFAULT_GROW_SIZE);
              strcpy (token + token_index, ttrans);
-- 
1.7.4.22.g14b16.dirty