[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Chris Bohn
Subject: Re: patch: under Windows, cvs status reports "memory exhausted"
Date: Thu, 25 Mar 2004 15:53:13 -0500
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

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
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);


/* 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:

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:

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;
                *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:

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:

expand_string (char **strptr, size_t *n, size_t
    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.


Derek Robert Price wrote:

Hash: SHA1

Chris Bohn wrote:

in cvs 1.12.6 client/server mode

This is also tracked in issue 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.


- --

Email: address@hidden

Get CVS support at <http://ximbiot.com>!
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Netscape - http://enigmail.mozdev.org


reply via email to

[Prev in Thread] Current Thread [Next in Thread]