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

[Chris Bohn]

x2realloc wasn't the problem; it was the wrapper function around it, 
expand_string, which is called from xrealloc_and_strcat.

You can look at the diff of the subr.c code that changed in cvs 1.12.6 
to see the differences:

revision 1.100
date: 2004/02/21 00:09:38;  author: dprice;  state: Exp;  lines: +2 -33
* subr.c (expand_string): Use x2realloc() from GNULIB for core
functionality.
----------------------------
revision 1.99
date: 2004/02/20 23:16:55;  author: dprice;  state: Exp;  lines: +0 -24
Merge changes from 1.11.x.

client.c does like this under Windows (in send_file_names):

/* Add the slash unless this is our first element. */
if (line_len)
    xrealloc_and_strcat (&line, &line_len, "/");
xrealloc_and_strcat (&line, &line_len, node->key);

and later

/* If node is still NULL then we either didn't find CVSADM or
* we didn't find an entry there.
*/
if (node == NULL)
{
    /* Add the slash unless this is our first element. */
    if (line_len)
        xrealloc_and_strcat (&line, &line_len, "/");
        xrealloc_and_strcat (&line, &line_len, q);
        break;
    }

and

/* Now put everything we didn't find entries for back on. */
while (q = pop (stack))
{
    if (line_len)
        xrealloc_and_strcat (&line, &line_len, "/");
    xrealloc_and_strcat (&line, &line_len, q);
    free (q);
}

Notice how it is relying on the passed in line_len parameter to be set 
to the new size when the function is done.

xrealloc_and_strcat looks like this in both versions of the file:

void
xrealloc_and_strcat (char **str, size_t *lenp, const char *src)
{

    expand_string (str, lenp, strlen (*str) + strlen (src) + 1);
    strcat (*str, src);
}

Remember the second parameter, lenp (line_len in the actual function 
call in client.c), is what client.c is expecting to be changed to be the 
new size.

In 1.99 of subr.c, expand_string looks like:

void
expand_string (char **strptr, size_t *n, size_t newsize)
{
    if (*n < newsize)
    {
        while (*n < newsize)
        {
            if (*n < MIN_INCR)
                *n = MIN_INCR;
            else if (*n >= MAX_INCR)
                *n += MAX_INCR;
            else
            {
                *n *= 2;
                if (*n > MAX_INCR)
                    *n = MAX_INCR;
            }
        }
        *strptr = xrealloc (*strptr, *n);
    }
}

So it is the second parameter here too, n, that needs to be reset to the 
new size for everything to work as before.  In this version of the 
function, *n is what is set to higher values until it is bigger than the 
requested new size, so when the function is done, that parameter is 
modified to be the new, ending size of the string.

In 1.100 of subr.c, first released with cvs 1.12.6, expand_string looks 
like:

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.  x2realloc is called passing 
in only the old length, which is initially 0 when passed down from 
client.c (send_file_names) in the first call.  0 will always be < 
newsize, which is a calculation using the real string lengths of the 
strings in question.  This means the while loop will never terminate 
because *n will always be less than newsize because *n is never changing 
anywhere in the new implementation.  When a non-zero n is passed in, 
x2realloc will keep reallocating until it runs out of memory.  My 
suggested fix makes the function implementation look like:

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

which makes the "while" an "if" just in case (and there is no reason to 
loop now using x2realloc), sends in newsize to x2realloc since that is 
the new size desired, and then updates the second parameter, *n, to be 
the newly reallocated size, which is what the calling functions were 
expecting.  As I indicated in the issue (not the email), this is only a 
problem under Windows because the code that causes the problem in 
client.c (calls to xrealloc_and_strcat) is in an #ifdef 
FILENAMES_CASE_INSENSITIVE block, and the same code isn't run on unix.

So I think I have everything correct.

Chris

Derek Robert Price wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris Bohn wrote:
>
>
> > in cvs 1.12.6 client/server mode
> >
> > This is also tracked in issue 168:
> > http://ccvs.cvshome.org/issues/show_bug.cgi?id=168
> >
> > I wanted to submit the patch to this list as well because that is the
> > way suggested in HACKING.  See the issue for full details, but the
> > patch is also pasted below.  I really think this should be in the next
> > release since Windows users can't run correctly without it.  The root
> > problem is in subr.c, but the problem affects nearly all subcommands
> > because the problem code is hit in send_file_names, which is called
> > for nearly everything.
>
>
>
> I'm not sure what is causing your problem on Windows, but I doubt it is
> this.  x2realloc(str, &size) is guaranteed to increase SIZE with each
> call.  This particular implementation doubles it and there should be no
> differences under Windows.
>
> Can you supply a stack trace for your failure case?
>
>
> > There is a whole testing framework for unix, but there is nothing for
> > Windows.  I think if there was any for Windows, this would have been
> > caught because doing a simple cvs status with a file name argument
> > exposes the problem.  I realize many of the developers of cvs do their
> > work on unix, but if Windows is going to be supported, then some type
> > of regression testing framework should probably be made for it too,
> > especially now with different code paths because of the case
> > sensitivity changes (where this bug was located).
>
>
>
> Yep.  This would be nice.  I've thought so for awhile.
>
> 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
>
> iD8DBQFAYzzvLD1OTBfyMaQRAhA8AJwNLtEXs8yg5t+8bq+2/64bNCs7dQCgxxpV
> eza+QFmOGCPMyWwW4egkGPw=
> =dXCe
> -----END PGP SIGNATURE-----