Martin Sebor wrote
>
>Travis Vitek wrote:
>>
>>
>>> Martin Sebor wrote:
>>>
>>> [EMAIL PROTECTED] wrote:
>>>> URL: http://svn.apache.org/viewvc?rev=647908&view=rev
>>>> Log:
>>>> 2008-04-14 Travis Vitek <[EMAIL PROTECTED]>
>>>>
>>>> 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
>>>
>>
>> I don't think I can use the maxsize field for this, but I
>> have a bit of confusion about this maxsize member.
>>
>> If maxsize != _RWSTD_UINT_MAX, then the user has provided
>> the necessary format specifier to tell us not to grow the
>> buffer past a set length. We are still allowed to grow the
>> buffer, just not past this limit. Since we are allowed to
>> grow the buffer, even if maxsize is set, I need some way to
>> indicate that I don't know who allocated the buffer or
>> where it came from.
>
>Wouldn't (*pbufsize == maxsize) in that case so that _rw_bufcat()
>would never need to allocate new space to begin with (or it would
>fail trying).
>
That only tells me if I've reached the maximum allowed buffer size. It
tells me nothing about where the *pbufsize bytes were allocated from,
and that is exactly what I need, right?
I mean there are cases where (*pbufsize != maxsize) and we don't want to
deallocate the buffer. Consider this case which is taken from the test
library...
static int
_rw_vsystem (const char *cmd, va_list va)
{
RW_ASSERT (0 != cmd);
char buffer [256];
char *buf = buffer;
size_t bufsize = sizeof buffer;
rw_vasnprintf (&buf, &bufsize, cmd, va);
// <snip>
}
We definitely don't want `buf' to be deallocated, that much is clear.
But how can `maxsize' possibly tell us anything about `buf'?
>
>>
>> Now I know that I could have stolen a bit from maxsize for a
>> flag, but I didn't want to do that. The additional size_t
>> field didn't seem all that expensive considering that these
>> buffer objects are allocated on the stack in a call to a
>> formatted output function.
>
>Right. I'm not worried about the space overhead of a new data
>member but rather about duplicating the same information in
>two places, and the two becoming out of sync.
>
Agreed.
>>
>> I just noticed that the code in fmt_bits.cpp uses rw_asnprintf()
>> expecting that the buffer reallocation done internally would free
>> the input buffer if necessary. Previously that might have caused
>> the assert that my change was supposed to fix. Now that code
>> might leak the buffer. So I'm putting together a patch to fix
>> that now.
>
>Yeah, the code there (the "%{+}" directives) is kind of clunky.
>
>>
>> In fixing that, I noticed that we have a function rw_vasnprintf() is
>> defined in printf.cpp, but it isn't declared in rw_printf.h. That
>> function is declared in each of tests/src/{ctype, driver, exception,
>> process}.cpp. For a clean fix I'd like to use the function in
>> test/src/fmt_bits.cpp, but I'd prefer to not forward declare
>> in another place. Is there some motivation to not declare the
>> function in the appropriate place. The only reason I can think
>> of to not do so would be introducing <stdarg.h> into one of the
>> test headers, but that doesn't seem like it should be a problem
>> because we are doing it in all of the previously mentioned source
>> files.
>
>That's the reason. <rw_printf.h> isn't supposed to depend on any
>libc (or C++ standard library) header so that it can be included
>in tests without introducing namespace pollution. This should be
>documented somewhere. But instead of declaring the function in
>every driver source file that uses it maybe we could declare it
>in a driver-only header?
>
Got it. I'm just going to fwd declare it this time. I'll file an
enhancement for this to be done at a later time.
>Martin
>
