Hi,
issue 109142 (http://www.openoffice.org/issues/show_bug.cgi?id=109142)
requests to change the behavior of assertions (OSL_ASSERT/DBG_ASSERT and
friends) to abort if their condition is not met. The current behavior is
that the assertion text is reported to the user, usually by a message box.
All of the above applies to non-product builds only, in particular, the
request, as I understand it, is *not* to extend assertions to product
builds.
Since this is a controversial topic, I think we should discuss it in a
wider audience before actually implementing the change - so, let's get
the party started ...
Actually, /me thinks we should *not* change the behavior as suggested.
An ::std::assert, which also aborts when hit, basically means something
like "My state is that inconsistent and unrecoverable, I can't
legitimately continue execution." In such a case, an abort is justified.
(However, in opposite to what seems to be suggested in the issue, this
case also justifies an abort in the product build, i.e. when running at
the customer's machine. Whether this is something a product like ours
should use more than to a negligible extent, is a separate question,
eligible for an even more controversial discussion).
However, OSL_*, and even more DBG_*, are a little bit different. In
particular, I always understood them (and I know a lot of others used
them that way, too) as "Something didn't go the way it was
intended/expected.". This surely indicates a bug which needs attention,
but it does *not necessarily* imply that an abort is the only legitimate
reaction. Contrary, a lot of code tries to get out of that concrete
situation gracefully, continuing with what still can be done. So, an
abort is not justified here, IMO.
When asking for the goal of the change, the issue says
Without such a strict measure, the many assertions occurring in a run
of OOo (...) would probably never go a away, and the assertion tool
would remain practically useless (...)
But making assertions abort instead of "just" being reported would
worsen the situation, IMO: We have that many assertions in non-product
builds because a) too few developers actually use non-product builds
(which means they silently introduce new ones) and b) fixing existing
assertions is not a priority [1].
Aborting in case of an assertion would just make a) much worse. Also, it
would put the burden onto the shoulders of those actually *using* the
non-product builds, not of those *causing* the pain. I.e., it would
punish the "wrong" people.
Having said all this, I'd say the mentioned issue should be a WONTFIX.
The problem which the issue tries to address, however, is still valid,
and highly legitimate, in my opinion. Assertions are a cheap and
already-existing measure to improve our product quality (and in my
personal opinion, both the 3.1 and the 3.2 release have shown that we
should improve here). Still, too few people use non-product builds,
which means new assertions are introduced, which means more people are
scared away from using non-product builds ... which renders assertions
nearly useless.
So, my suggestions would be the following (admittedly, they're more on a
social level, than on a technical one, and thus harder to enforce):
- Developers should use non-product builds *only*. That's a very
apparent measure, still, a lot of people don't do. If you ask why,
often the answer is "it's unusable 'cause of the many assertions".
Hmm?
- QA should use non-product builds *only*. Yes, I am not kidding about
this. An assertion which fails indicates a *bug*, that's the very
original intention of assertions: report bugs. Speaking strictly,
QA which refuses to use non-product builds refuses to do their job,
which at least in parts consists of finding bugs.
- Report assertions as you find them, and fix them as you get the issues
assigned. Give them high priority, as every assertion might be a
little effort for you to fix, but it is a lot of paint for a lot of
other people as long as it is not fixed.
- Get *serious* management backup for the three previous items.
(<insider>That's something which is highly missing since MA left years
ago :) </insider>)
- Provide non-product builds for release code lines, too. At least do so
much longer than we did with OOO320 (This item strictly applies to
Sun-internal workflows.):
For OOO320, we stopped providing non-product builds at branch-off day,
which was about half a year before the product was actually released.
During a significant part of this time, there still happened heavy
development on that code line, which is prone to introducing new
assertion failures, going unnoticed in product builds.
- Basically: Understand that assertions are not there to hinder your
work. They exist to help you (at pretty low costs) finding bugs in
your code, and thus improving our product - which is something we all
should be interested in, right?
- Consequently, *use* assertions when writing new code. There's no such
thing as "too much assertions" in the code - as long as they never
fire, of course :)
Ciao
Frank
[1] Most of the assertions during smoketest were submitted months ago,
each time targeted to the next minor release. Some of them have been
re-targeted twice meanwhile.
--
- Frank Schönheit, Software Engineer frank.schoenh...@sun.com -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Base http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@openoffice.org
For additional commands, e-mail: dev-h...@openoffice.org
