Re: abort() calls

Branko Čibej wrote:
> Stefan Küng wrote:
>> Hi,
>>
>> In the Subversion code, there are a lot of calls to the abort()
>> function. I think those should be removed and replaced with something
>> else.
>
> Oh wow, you've gone and opened a can of worms that we've had trouble
> closing in the past.

Well, I think in this case, that can isn't really closed :)

> A note about assert vs. abort: As long as it remains common practice on
> Windows to compile release binaries with NDEBUG defined, assert is worse
> than useless.

You're controlling how the Subversion libs are built with the build
script. So even if the app linking to the Subversion libs has NDEBUG
defined, the lib still would have the assert() correctly compiled in?

> Also, returning an error implies that we can (somehow) recover from it,
> which isn't always the case.

That depends on who you ask I think. I'd rather have an error message
even if it means it's not possible to recover from it:
showing an error and then crash completely is much better than a useless
message and shutdown.
And I've seen a lot of places where it's possible to recover (it won't
be possible to do the task which was requested, but that happens all the
time when an error is returned instead of the requested value). But
still, abort() is called. It looks as if a call to abort() seemed easier
at the time the code was written than to introduce another error define.
Just check some of the examples of my previous mail.

> We probably do call abort() in places where we could do something else;
> such bits can be fixed. But we decided long ago on the policy to not
> cater to clients that violate the API contract, so returning an error in
> those cases would mean changing that policy. Nothing wrong with that in
> general, it's just yet another can of worms.

I don't understand. Where's the API contract that says that the API
could simply abort the whole program? Ok, for the server part I can
understand that it's better to abort than to throw an error and not be
able to recover. But for the client part, that's IMHO not an option.
 From my point of view, an API should *never* abort or crash or
something like that but always return an error. It's the job of the
client/server app to decide whether to abort or not.
What may seem the best solution for e.g. the server part may be the
worst for a client.

> I'm willing to consider using something else instead of abort(). But
> someone will have to come up with a proposal that works everywhere, not
> just in Windows GUI with just-in-time debugging.

Maybe there just isn't a solution for everything. I would suggest using
some define to let the app itself decide which behavior to get. For example:

#ifdef SVN_USE_ABORT
#define emerg_exit abort
#elif SVN_USE_ASSERT
#define emerg_exit assert
#endif

and then use
emerg_exit();
in the code. Or something else. That's just a suggestion. But with
abort(), I can't get any useful information if something goes wrong.

And yes, I think assert() is bad too. It's just a little less evil than
abort().

Try to understand my situation here: TortoiseSVN is embedded in the
windows shell. A call to abort() will shut down the shell. With an
assert, I could maybe get some information back from users who
experience this, but it's still very bad. In case an error is returned,
I can handle the situation myself in the shell extension and decide what
to do - but definitely not to shut the shell down or crash. In case of a
really bad situation, I could at least show a dialog informing the user
that he should save all work before clicking on the OK button, and
*then* shut down the shell or something like that. But I really hope
such situations don't exist and that it's possible to always return from
an API somehow.
For the command line client, an abort() is completely acceptable because
whether the program exits with a defined error or with an 'emergency'
error, the program exits anyway.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org