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 20:24:23 GMT
Travis Vitek wrote:
>  
> 
>> Martin Sebor wrote:
>>
>> vitek@apache.org wrote:
>>> 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
>>
> 
> 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 (i.e. a leading %{*} or %{n}) 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).

> 
> 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.

> 
> 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?

Martin

Mime
View raw message