Re: [jira] [Updated] (STDCXX-1058) std::basic_ios::copyfmt() with registered callback (via std::ios_base::register_callback()) run-time SIGABRT
On Mon, Jun 11, 2012 at 12:35 PM, Martin Sebor mse...@gmail.com wrote:
...
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.
Here's what I meant:
struct A: std::streambuf { };
struct B: std::ios {
A sb;
B () { init (sb); }
} f0, f1;
Btw., in your new test, either the TEST_ASSERT() macro should abort
or the test should when before returning ret is non-zero.
I.e., every regression test should report failure by calling abort
(via the assert() macro).
Returning a non-zero exit status (in addition to making use of
assert()) is fine but it shouldn't be the sole mechanism for
reporting an error.
Also, please avoid #including iostream in tests unless
exercising the standard streams (std::cout et al.) The header
runs complicated code and can lead to unrelated failures.
Attached is a slightly modified test to show what I mean. (Also
fixes formatting -- please use 4 space indents and a space before
each open paren; curly brace goes on the same line as the statement
except for namespace scope declarations).
Martin
Done - new attachment 27.basic_ios.copyfmt.stdcxx-1058.cpp
--Stefan
--
Stefan Teleman
KDE e.V.
stefan.tele...@gmail.com
Re: [jira] [Updated] (STDCXX-1058) std::basic_ios::copyfmt() with registered callback (via std::ios_base::register_callback()) run-time SIGABRT
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 Sebormse...@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
Re: [jira] [Updated] (STDCXX-1058) std::basic_ios::copyfmt() with registered callback (via std::ios_base::register_callback()) run-time SIGABRT
On 05/16/2012 09:23 PM, Stefan Teleman wrote:
On Wed, May 16, 2012 at 2:58 PM, Martin Sebormse...@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.
Martin
--Stefan
