On Thu, May 17, 2012 at 11:58 AM, Martin Sebor <mse...@gmail.com> wrote:
> On 05/16/2012 09:23 PM, Stefan Teleman wrote:
>>
>> On Wed, May 16, 2012 at 2:58 PM, Martin Sebor<mse...@gmail.com> wrote:
>>>
>>> On 05/16/2012 11:55 AM, Travis Vitek wrote:
>>>>
>>>>
>>>> I approve the change, but with one caveat. The branching policy [1]
>>>> indicates that you should commit your changes directly to the 4.2.x
>>>> branch.
>>>> They should be merged from 4.2.x to 4.3.x, and then from 4.3.x to trunk.
>>>
>>>
>>>
>>> The test in the issue should be committed with the patch (after
>>> adding the necessary asserts, being renamed to follow the naming
>>> convention for regression tests, i.e., something like
>>> regress/27.basic_ios.stdcxx-1058.cpp, and decorated with the ASF
>>> license header).
>>
>>
>> I attached 27.ios_base.event_callback.stdcxx-1058.cpp to stdcxx-1058,
>> which will go in ${TOPDIR}/test/regress/.
>
>
> Great! A few suggestions:
>
> The naming convention used for the tests is based on a few things:
>
> 1. the section number in the standard of the tested functionality
> (this is only used for sorting)
> 2. the name of class whose members are being tested (if any)
> 3. the name of the function or type (or member function or type)
> being tested (may be omitted for tests that exercise multiple
> members)
> 4. for regression tests, the issue number
>
> Since this issue is about std::basic_ios::copyfmt() crashing (as
> the subject says), its name should probably look something like:
>
> 27.basic_ios.copyfmt.stdcxx-1056.cpp
>
> Each test should exercise just the affected class, function, or
> type. It's best to avoid relying on parts of the library that
> aren't affected or subject of the test so that when they break
> as a result of a some unrelated change in the future we don't
> start seeing failures in unrelated tests. In this case, I would
> suggest to avoid using fstream (and especially cerr) and rely
> directly on basic_ios (define a derived class to access its
> protected ctor). Most regression tests don't tend to produce
> debugging output but if you find it useful please use stdio,
> not iostreams.
>
> Finally, regression tests should verify expected postconditions
> by using the assert() macro. The exit status can be 0 on success
> and non-zero on failure, but this is not done consistently.
OK I attached a new test case - 27.basic_ios.copyfmt.stdcxx-1058.cpp.
But, using a simple derived class from std::basic_ios doesn't trigger
the bug. It's only triggered when using std::fstream or
std::stringstream.
--Stefan
--
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com
