stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r647908 - in /stdcxx/trunk/tests: self/0.printf.cpp src/fmt_defs.h src/printf.cpp
Date Mon, 14 Apr 2008 18:01:04 GMT
vitek@apache.org wrote:
> Author: vitek
> Date: Mon Apr 14 10:15:49 2008
> New Revision: 647908
> 
> URL: http://svn.apache.org/viewvc?rev=647908&view=rev
> Log:
> 2008-04-14  Travis Vitek  <vitek@roguewave.com>
> 
> 	STDCXX-857
> 	* tests/src/fmt_defs.h: Add flag to struct Buffer to indicate
> 	who owns the allocated buffer.

I wonder if this flag is really necessary give the maxsize
member of Buffer whose purpose was to avoid reallocation
(the implementation may have fallen short of that goal):
http://svn.apache.org/viewvc?view=rev&revision=381880

Martin

> 	* tests/src/printf.cpp (_rw_bufcat): Simplify logic to get
> 	new buffer size. Avoid checking guard bytes and deallocating
> 	buffer that we do not own.
> 	(rw_vasnprintf): Set flag indicating that the caller owns the
> 	supplied buffer.
> 	(_rw_fmtarray): Ditto.
> 	(_rw_fmt_expr): Ditto.
> 	* tests/self/0.printf.cpp (test_reallocate): Add new test to
> 	verify buffer reallocation works as expected.
> 
> Modified:
>     stdcxx/trunk/tests/self/0.printf.cpp
>     stdcxx/trunk/tests/src/fmt_defs.h
>     stdcxx/trunk/tests/src/printf.cpp
> 
> Modified: stdcxx/trunk/tests/self/0.printf.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/self/0.printf.cpp?rev=647908&r1=647907&r2=647908&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/self/0.printf.cpp (original)
> +++ stdcxx/trunk/tests/self/0.printf.cpp Mon Apr 14 10:15:49 2008
> @@ -3279,6 +3279,38 @@
>  
>  /***********************************************************************/
>  
> +static void
> +test_reallocate ()
> +{
> +    static const char abc[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +
> +    char buffer [4] = { 0 };
> +    char *buf = buffer;
> +
> +    size_t bufsize = sizeof buffer;
> +
> +    const int n = rw_asnprintf (&buf, &bufsize, "%s", abc);
> +
> +    if (n != 26 || strcmp (abc, buf)) {
> +        ++nfailures;
> +
> +        fprintf (stderr, "# Assertion failed on line %d: "
> +                         "rw_asnprintf(...) == \"%s\", got \"%s\"\n",
> +                         __LINE__, abc, buf);
> +    }
> +
> +    if (buf != buffer)
> +        free (buf);
> +    else {
> +        ++nfailures;
> +
> +        fprintf (stderr, "# Assertion failed on line %d: "
> +                         "rw_asnprintf(...) failed to reallocate buffer\n",
> +                         __LINE__);
> +    }
> +}
> +
> +
>  int main ()
>  {
>      test_percent ();
> @@ -3326,6 +3358,8 @@
>      test_nested_format ();
>  
>      test_malformed_directives ();
> +
> +    test_reallocate ();
>  
>      //////////////////////////////////////////////////////////////////
>      if (nfailures) {
> 
> Modified: stdcxx/trunk/tests/src/fmt_defs.h
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/fmt_defs.h?rev=647908&r1=647907&r2=647908&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/src/fmt_defs.h (original)
> +++ stdcxx/trunk/tests/src/fmt_defs.h Mon Apr 14 10:15:49 2008
> @@ -59,6 +59,7 @@
>      size_t  *pbufsize;   // pointer to the size of the buffer
>      size_t   maxsize;    // maximum not-to-exceed size
>      size_t   endoff;     // offset of the last character
> +    size_t   owned;      // buffer is owned by caller, don't free it
>  };
>  
>  /********************************************************************/
> 
> Modified: stdcxx/trunk/tests/src/printf.cpp
> URL: http://svn.apache.org/viewvc/stdcxx/trunk/tests/src/printf.cpp?rev=647908&r1=647907&r2=647908&view=diff
> ==============================================================================
> --- stdcxx/trunk/tests/src/printf.cpp (original)
> +++ stdcxx/trunk/tests/src/printf.cpp Mon Apr 14 10:15:49 2008
> @@ -450,26 +450,24 @@
>  
>          size_t guardsize = sizeof guard - 1;
>  
> -        size_t newbufsize = *buf.pbufsize * 2 + guardsize;
> +        size_t newbufsize = *buf.pbufsize * 2;
>  
> -        if (newbufsize <= buflen + len + guardsize)
> -            newbufsize = 2 * (buflen + len + 1) + guardsize;
> +        const size_t required = buflen + len;
> +        if (newbufsize < required)
> +            newbufsize = required * 2;
>  
>          // prevent buffer size from exceeding the maximum
>          if (buf.maxsize < newbufsize)
>              newbufsize = buf.maxsize;
>  
> -        if (newbufsize < buflen + len + guardsize)
> -            guardsize = 0;
> -
> -        if (newbufsize < buflen + len + guardsize) {
> +        if (newbufsize < required) {
>  #ifdef ENOMEM
>              errno = ENOMEM;
>  #endif   // ENOMEM
>              return 0;
>          }
>  
> -        char* const newbuf = (char*)malloc (newbufsize);
> +        char* const newbuf = (char*)malloc (newbufsize + guardsize);
>  
>          // return 0 on failure to allocate, let caller deal with it
>          if (0 == newbuf)
> @@ -478,17 +476,21 @@
>          memcpy (newbuf, *buf.pbuf, buflen);
>  
>          // append a guard block to the end of the buffer
> -        memcpy (newbuf + newbufsize - guardsize, guard, guardsize);
> +        memcpy (newbuf + newbufsize, guard, guardsize);
>  
> -        if (*buf.pbuf) {
> +        if (*buf.pbuf && !buf.owned) {
>              // verify that we didn't write past the end of the buffer
>              RW_ASSERT (0 == memcmp (*buf.pbuf + *buf.pbufsize,
>                                      guard, guardsize));
> +
>              free (*buf.pbuf);
>          }
>  
>          *buf.pbuf     = newbuf;
> -        *buf.pbufsize = newbufsize - guardsize;
> +        *buf.pbufsize = newbufsize;
> +
> +        // we allocated the buffer, so we can free it
> +        buf.owned = 0;
>      }
>  
>      if (0 != str) {
> @@ -958,7 +960,7 @@
>      if (0 == pbufsize)
>          pbufsize = &default_bufsize;
>  
> -    Buffer buf = { pbuf, pbufsize, _RWSTD_SIZE_MAX, 0 };
> +    Buffer buf = { pbuf, pbufsize, _RWSTD_SIZE_MAX, 0, 1 };
>  
>      // save the initial value of `pbuf'
>      char* const pbuf_save = *buf.pbuf;
> @@ -1995,7 +1997,7 @@
>                  size_t localbufsize = sizeof elemstr;
>  
>                  Buffer bufspec = {
> -                    &localbuf, &localbufsize, sizeof elemstr, 0
> +                    &localbuf, &localbufsize, sizeof elemstr, 0, 1
>                  };
>  
>                  *localbuf = '\0';
> @@ -2053,7 +2055,7 @@
>                  size_t localbufsize = sizeof elemstr;
>  
>                  Buffer bufspec = {
> -                    &localbuf, &localbufsize, sizeof elemstr, 0
> +                    &localbuf, &localbufsize, sizeof elemstr, 0, 1
>                  };
>  
>                  *localbuf = '\0';
> @@ -2983,7 +2985,7 @@
>          const char* const fmt = VA_ARG (*pva, char*);
>  
>          size_t dummy_size = 0;   // unused
> -        Buffer tmpbuf = { &fmtword, &dummy_size, _RWSTD_SIZE_MAX, 0 };
> +        Buffer tmpbuf = { &fmtword, &dummy_size, _RWSTD_SIZE_MAX, 0, 1 };
>          const int len = _rw_pvasnprintf (tmpbuf, fmt, pva);
>          if (len < 0)
>              return -1;
> 
> 


Mime
View raw message