[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

abort() calls

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2007-04-28 20:04:03 CEST

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.

An abort() will close the application. There's no way to find out why
the application just quit - no usefull error message ("This application
has requested the Runtime to terminate it in an unusual way. Please
contact the application's support team for more information."), no
assertion which could at least help if a debugger is installed. If there
really is no other way to recover from a certain situation in the code
(not even returning an error), then please call assert(false) instead of
abort()!

And some of the places where I found a call to abort() could simply
return an error:

libsvn_client/commit.c: line 1296
                   if ((target[0] == '\0') ||
                       svn_dirent_is_root(target, strlen(target))
                      )
                     abort();
Seriously? Why not just return an error telling that a root is not allowed?

libsvn_diff/token.c: line 77
   if (!token)
     abort();

An error telling that an empty token was found would be much better.

libsvn_ra_dav/replay.c: line 218
             else
               abort();
An error indicating that an unknown element was read would at least
indicate where to look to solve the problem.

libsvn_ra_serf/blame.c: line 350
             svn_stream_write(info->stream, data, &ret_len);
             if (ret_len != len)
               {
                 abort();
               }
An error about an network error, that not all the data could be sent?

I especially don't like an abort() call in the wc part of the library,
because that's the part which TSVN uses the most in its shell extension
part - and an abort there will also close the explorer/desktop!
libsvn_wc/entries.c: line 1868
       /* This is NOT the "this dir" entry */
       if (! strcmp(name, "."))
         {
           /* By golly, if this isn't recognized as the "this dir"
              entry, and it looks like '.', we're just asking for an
              infinite recursion to happen. Abort! */
           abort();
         }
What happens if the working copy somehow got corrupted? In case of TSVN,
  that's the worst that could happen: the working copy is corrupted, but
the user can't browse with the explorer there to delete the folder,
because the explorer will shut down immediately when showing that folder.

libsvn_wc/status.c: line 1219
   /* Don't do this. Just do NOT do this to me. */
   if (pb && (! path))
     abort();

Yes, I agree: don't do this. Just do NOT do this to me!

There are a lot more places where abort() is called. I just showed some
of the worst here which could easily be replaced by returning a useful
error.

Remember that UI clients don't behave like a console application. And if
the Subversion library is used as a plugin to some other program, you
just are not allowed to simply call abort() if you can avoid it! Imagine
an IDE-plugin which shuts down the whole IDE, while the user hasn't
saved all changes yet. Or (again, I know I'm repeating myself) like TSVN
which brings down the whole desktop.

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
Received on Sat Apr 28 20:04:19 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.