Re: patch: under Windows, cvs status reports "memory exhausted"

[Chris Bohn]

I executed
cvs -d :pserver:admiral:/zip/tmp/cbohn/cvstest stat subr.c
from within the cvs src directory.

On the first invocation of xrealloc_and_strcat

xrealloc_and_strcat (&line, &line_len, node->key);

line 4318 of client.c (cvs 1.12.6 source)

line = ""
line_len = 0
node->key = subr.c

That calls causes the error.

Upon getting into expand_string from xrealloc_and_strcat:

expand_string (char **strptr, size_t *n, size_t newsize)

*strptr = ""
*n = 0
newsize = 7 (calculated in xrealloc_and_strcat from strlen (*str) + 
strlen (src) + 1 --> 0 + 6 + 1)

So we now have:
while (*n < newsize)
being
while (0 < 7)

in the full function:

void
expand_string (char **strptr, size_t *n, size_t newsize)
{
    while (*n < newsize)
        *strptr = x2realloc (*strptr, n);
}


The x2realloc call is the one that dies.

x2realloc calls x2nrealloc_inline (p, pn, 1);
being logically equivalent to x2nrealloc_inline ("", 0, 1);

Here is that function:
static inline void *
x2nrealloc_inline (void *p, size_t *pn, size_t s)
{
  size_t n = *pn;

  if (! p)
    {
      if (! n)
        {
          /* The approximate size to use for initial small allocation
             requests, when the invoking code specifies an old size of
             zero.  64 bytes is the largest "small" request for the
             GNU C library malloc.  */
          enum { DEFAULT_MXFAST = 64 };

          n = DEFAULT_MXFAST / s;
          n += !n;
        }
    }
  else
    {
      if (SIZE_MAX / 2 / s < n)
        xalloc_die ();
      n *= 2;
    }

  *pn = n;
  return xrealloc (p, n * s);
}

removing inline so I could debug into in the IDE, it revealed it hits 
the else of "if (! p)" because p was allocated back in client.c:

char *line = xmalloc (1);
*line = '\0';

so in this case, the if above the xalloc_die isn't called because the 
expr is > 0, so n and *pn get set to 0*2 = 0 still.

xrealloc (p, n * s) gets called with "", 0*1 --> xrealloc ("", 0)
that calls
xnrealloc_inline (p, n, 1);
and xalloc_die is called from
if (xalloc_oversized (n, s) || ! (p = realloc (p, n * s)))

xalloc_oversized is false, but since realloc is called like
realloc(p, 0), it reallocs the block to 0, and !0 = 1, so it dies.  I 
thought I would step through the runtime a little more, and I found:

        /*
         * ANSI: realloc(pUserData, 0) is equivalent to free(pUserData)
         * (except that NULL is returned)
         */
        if (fRealloc && nNewSize == 0)
        {
            _free_dbg(pUserData, nBlockUse);
            return NULL;
        }

so that explains the problem.  I apologize for not digging in this much 
initially, but when initially debugging, the debugger wasn't going where 
I expected because of the inline calls, so I just thought something was 
messed up and ignored the problem.  So the solution to this problem is 
debatable.  Should client.c not xmalloc(1), should x2nrealloc_inline be 
smarter and not just check !p, etc.?  Changing x2nrealloc_inline sounds 
like the best way to me.  You could do something like checking for a 
zero len string and size allocated only being 1, but you have to cast 
everything from the void *, which likely isn't desirable.  I can't 
really find any docs on what these x2 functions are supposed to do 
(exact specs), so I don't know if they should change or if the caller 
(client.c) should have known the proper usage.  Either way, it is a bad 
error coming out of x2realloc, so that should change at a minimum to 
return a better error if 0 isn't valid with an already allocated string. 
 My fix will still work, but it is allocating more than it needs in the 
case of already allocated string and 0 current size.  If all stays as 
is, xrealloc_and_strcat (in subr.c) should change to not calculate and 
send newsize because it isn't needed or used if x2realloc starts with 
the original size (lenp).

Chris


Derek Robert Price wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris Bohn wrote:
>
>
> > x2realloc wasn't the problem; it was the wrapper function around it,
> > expand_string, which is called from xrealloc_and_strcat.
>
>
>
> x2realloc (str, size) doubles *size (setting it in the process), and
> returns xrealloc (str, *size).  This is exactly what expand_string()
> used to be doing.
>
> There should also be no difference in the behavior of expand_string() or
> x2realloc() between UNIX & Windows.  Can you provide a debugger stack
> trace for the `cvs status' failure you reported?
>
> Once again:
>
>
> > void
> > expand_string (char **strptr, size_t *n, size_t newsize)
> > {
> >    while (*n < newsize)
> >    *strptr = x2realloc (*strptr, n);
> > }
> >
> > The first line will likely always be true because this function is
> > called to append two strings in this case.  
>
>
>
> No, it won't.  In your example above, x2realloc(*strptr, n) doubles *n
> with each invocation.  Please provide a stack trace.
>
> Derek
>
> - --
>                 *8^)
>
> Email: derek@ximbiot.com
>
> Get CVS support at <http://ximbiot.com>!
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.1 (GNU/Linux)
> Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org
>
> iD8DBQFAY0kNLD1OTBfyMaQRAjpXAJ4qnRzoDgLnNVAHO5kxRee9Zz4PHwCfaQ8s
> 92LRHhNFESk5mUApftthq2k=
> =d9f4
> -----END PGP SIGNATURE-----