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

[PATCH] abort or assert?

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2004-11-09 19:37:46 CET

This patch converts aborts into asserts where it seems most appropriate.

- Julian

Use "assert" where appropriate instead of "if (condition) abort();",
throughout the code.

Index: subversion/libsvn_fs_base/reps-strings.c
===================================================================
--- subversion/libsvn_fs_base/reps-strings.c (revision 11796)
+++ subversion/libsvn_fs_base/reps-strings.c (working copy)
@@ -421,12 +421,13 @@
 
   /* Read in our REP. */
   SVN_ERR (svn_fs_bdb__read_rep (&rep, fs, rep_key, trail));
+ assert (rep->kind == rep_kind_delta || rep->kind == rep_kind_fulltext);
   if (rep->kind == rep_kind_fulltext)
     {
       SVN_ERR (svn_fs_bdb__string_read (fs, rep->contents.fulltext.string_key,
                                         buf, offset, len, trail));
     }
- else if (rep->kind == rep_kind_delta)
+ else
     {
       const int cur_chunk = get_chunk_offset (rep, offset, &chunk_offset);
       if (cur_chunk < 0)
@@ -463,17 +464,14 @@
 
           /* Right. We've either just read the fulltext rep, a rep that's
              too short, in which case we'll undeltify without source data.*/
- if (rep->kind != rep_kind_delta && rep->kind != rep_kind_fulltext)
- abort(); /* unknown kind */
-
+ assert (rep->kind == rep_kind_delta
+ || rep->kind == rep_kind_fulltext);
           if (rep->kind == rep_kind_delta)
             rep = NULL; /* Don't use source data */
           SVN_ERR (rep_undeltify_range (fs, reps, rep, cur_chunk,
                                         buf, chunk_offset, len, trail));
         }
     }
- else /* unknown kind */
- abort ();
 
   return SVN_NO_ERROR;
 }
@@ -526,20 +524,19 @@
   if (! rep_is_mutable (rep, txn_id))
     return SVN_NO_ERROR;
 
+ assert (rep->kind == rep_kind_delta || rep->kind == rep_kind_fulltext);
   if (rep->kind == rep_kind_fulltext)
     {
       SVN_ERR (svn_fs_bdb__string_delete (fs,
                                           rep->contents.fulltext.string_key,
                                           trail));
     }
- else if (rep->kind == rep_kind_delta)
+ else
     {
       apr_array_header_t *keys;
       SVN_ERR (delta_string_keys (&keys, rep, trail->pool));
       SVN_ERR (delete_strings (keys, fs, trail));
     }
- else /* unknown kind */
- abort ();
 
   SVN_ERR (svn_fs_bdb__delete_rep (fs, rep_key, trail));
   return SVN_NO_ERROR;
@@ -646,6 +643,7 @@
 
   SVN_ERR (svn_fs_bdb__read_rep (&rep, fs, rep_key, trail));
 
+ assert (rep->kind == rep_kind_delta || rep->kind == rep_kind_fulltext);
   if (rep->kind == rep_kind_fulltext)
     {
       /* Get the size by asking Berkeley for the string's length. */
@@ -653,7 +651,7 @@
                                         rep->contents.fulltext.string_key,
                                         trail));
     }
- else if (rep->kind == rep_kind_delta)
+ else
     {
       /* Get the size by finding the last window pkg in the delta and
          adding its offset to its size. This way, we won't even be
@@ -668,8 +666,6 @@
                                   rep_delta_chunk_t *);
       *size_p = last_chunk->offset + last_chunk->size;
     }
- else /* unknown kind */
- abort ();
 
   return SVN_NO_ERROR;
 }
@@ -949,12 +945,13 @@
       (SVN_ERR_FS_REP_NOT_MUTABLE, NULL,
        "Rep '%s' is not mutable", rep_key);
 
+ assert (rep->kind == rep_kind_delta || rep->kind == rep_kind_fulltext);
   if (rep->kind == rep_kind_fulltext)
     {
       SVN_ERR (svn_fs_bdb__string_append
                (fs, &(rep->contents.fulltext.string_key), len, buf, trail));
     }
- else if (rep->kind == rep_kind_delta)
+ else
     {
       /* There should never be a case when we have a mutable
          non-fulltext rep. The only code that creates mutable reps is
@@ -963,8 +960,6 @@
         (SVN_ERR_FS_CORRUPT, NULL,
          "Rep '%s' both mutable and non-fulltext", rep_key);
     }
- else /* unknown kind */
- abort ();
 
   return SVN_NO_ERROR;
 }
@@ -1412,6 +1407,8 @@
     const char *str_key;
 
     SVN_ERR (svn_fs_bdb__read_rep (&old_rep, fs, target, trail));
+ assert (old_rep->kind == rep_kind_delta
+ || old_rep->kind == rep_kind_fulltext);
     if (old_rep->kind == rep_kind_fulltext)
       {
         svn_filesize_t old_size = 0;
@@ -1434,10 +1431,8 @@
             return SVN_NO_ERROR;
           }
       }
- else if (old_rep->kind == rep_kind_delta)
+ else
       SVN_ERR (delta_string_keys (&orig_str_keys, old_rep, pool));
- else /* unknown kind */
- abort ();
 
     /* Save the checksum, since the new rep needs it. */
     memcpy (rep_digest, old_rep->checksum, APR_MD5_DIGESTSIZE);
Index: subversion/libsvn_fs_base/trail.c
===================================================================
--- subversion/libsvn_fs_base/trail.c (revision 11796)
+++ subversion/libsvn_fs_base/trail.c (working copy)
@@ -15,6 +15,8 @@
  * ====================================================================
  */
 
+#include <assert.h>
+
 #define APU_WANT_DB
 #include <apu_want.h>
 
@@ -105,10 +107,9 @@
   trail->undo = 0;
   if (use_txn)
     {
- /* If we're already inside a trail operation, abort() -- this is
+ /* If we're already inside a trail operation, abort -- this is
          a coding problem (and will likely hang the repository anyway). */
- if (bfd->in_txn_trail)
- abort();
+ assert (! bfd->in_txn_trail);
 
       SVN_ERR (BDB_WRAP (fs, "beginning Berkeley DB transaction",
                          bfd->env->txn_begin (bfd->env, 0,
Index: subversion/libsvn_fs_base/util/skel.c
===================================================================
--- subversion/libsvn_fs_base/util/skel.c (revision 11796)
+++ subversion/libsvn_fs_base/util/skel.c (working copy)
@@ -15,6 +15,7 @@
  * ====================================================================
  */
 
+#include <assert.h>
 #include <string.h>
 #include "svn_string.h"
 #include "skel.h"
@@ -361,8 +362,7 @@
           int length_len;
 
           length_len = svn_fs_base__putsize (buf, sizeof (buf), skel->len);
- if (! length_len)
- abort ();
+ assert (length_len);
 
           /* Make sure we have room for the length, the space, and the
              atom's contents. */
@@ -443,8 +443,7 @@
 {
   /* If list_skel isn't even a list, somebody's not using this
      function properly. */
- if (list_skel->is_atom)
- abort();
+ assert (! list_skel->is_atom);
 
   skel->next = list_skel->children;
   list_skel->children = skel;
@@ -456,8 +455,7 @@
 {
   /* If list_skel isn't even a list, somebody's not using this
      function properly. */
- if (list_skel->is_atom)
- abort();
+ assert (! list_skel->is_atom);
 
   /* No kids? Let's make one. */
   if (! list_skel->children)
Index: subversion/libsvn_fs_base/util/fs_skels.c
===================================================================
--- subversion/libsvn_fs_base/util/fs_skels.c (revision 11796)
+++ subversion/libsvn_fs_base/util/fs_skels.c (working copy)
@@ -15,6 +15,7 @@
  * ====================================================================
  */
 
+#include <assert.h>
 #include <string.h>
 #include "svn_error.h"
 #include "svn_string.h"
@@ -961,6 +962,7 @@
 
   /** Do the kind-specific stuff. **/
 
+ assert (rep->kind == rep_kind_delta || rep->kind == rep_kind_fulltext);
   if (rep->kind == rep_kind_fulltext)
     {
       /*** Fulltext Representation. ***/
@@ -980,7 +982,7 @@
       /* header */
       svn_fs_base__prepend (header_skel, skel);
     }
- else if (rep->kind == rep_kind_delta)
+ else
     {
       /*** Delta Representation. ***/
       int i;
@@ -1046,8 +1048,6 @@
       /* header */
       svn_fs_base__prepend (header_skel, skel);
     }
- else /* unknown kind */
- abort();
 
   /* Validate and return the skel. */
   if (! is_valid_representation_skel (skel))
Index: subversion/libsvn_diff/token.c
===================================================================
--- subversion/libsvn_diff/token.c (revision 11796)
+++ subversion/libsvn_diff/token.c (working copy)
@@ -17,6 +17,8 @@
  */
 
 
+#include <assert.h>
+
 #include <apr.h>
 #include <apr_pools.h>
 #include <apr_general.h>
@@ -76,8 +78,7 @@
   svn_diff__node_t *parent;
   int rv;
 
- if (!token)
- abort();
+ assert (token);
 
   parent = NULL;
   node_ref = &tree->root[hash % SVN_DIFF__HASH_SIZE];
Index: subversion/libsvn_wc/entries.c
===================================================================
--- subversion/libsvn_wc/entries.c (revision 11796)
+++ subversion/libsvn_wc/entries.c (working copy)
@@ -1026,13 +1026,11 @@
   if (strcmp (name, SVN_WC_ENTRY_THIS_DIR))
     {
       /* 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();
- }
+
+ /* 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! */
+ assert (strcmp (name, ".") != 0);
 
       if (entry->kind == svn_node_dir)
         {
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 11796)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -946,8 +946,7 @@
   svn_wc_status_t *parent_status;
 
   /* Don't do this. Just do NOT do this to me. */
- if (pb && (! path))
- abort();
+ assert (!pb || path);
 
   /* Construct the full path of this directory. */
   if (pb)
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 11796)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -269,8 +269,7 @@
   struct bump_dir_info *bdi;
   
   /* Don't do this. Just do NOT do this to me. */
- if (pb && (! path))
- abort();
+ assert (!pb || path);
 
   /* Construct the PATH and baseNAME of this directory. */
   d->path = apr_pstrdup (pool, eb->anchor);
@@ -547,8 +546,7 @@
   struct file_baton *f = apr_pcalloc (pool, sizeof (*f));
 
   /* I rather need this information, yes. */
- if (! path)
- abort();
+ assert (path);
 
   /* Make the file's on-disk name. */
   f->path = svn_path_join (pb->edit_baton->anchor, path, pool);
@@ -1009,9 +1007,7 @@
 
   /* Semantic check. Either both "copyfrom" args are valid, or they're
      NULL and SVN_INVALID_REVNUM. A mixture is illegal semantics. */
- if ((copyfrom_path && (! SVN_IS_VALID_REVNUM (copyfrom_revision)))
- || ((! copyfrom_path) && (SVN_IS_VALID_REVNUM (copyfrom_revision))))
- abort();
+ assert ((copyfrom_path != NULL) == SVN_IS_VALID_REVNUM (copyfrom_revision));
 
   /* There should be nothing with this name. */
   SVN_ERR (svn_io_check_path (db->path, &kind, db->pool));
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 11796)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -1162,8 +1162,7 @@
           target = parent_dir;
           while (strcmp (target, base_dir))
             {
- if (target[0] == '/' && target[1] == '\0')
- abort();
+ assert (! (target[0] == '/' && target[1] == '\0'));
 
               APR_ARRAY_PUSH (dirs_to_lock,
                               const char *) = apr_pstrdup (pool, target);
Index: subversion/tests/libsvn_fs_base/skel-test.c
===================================================================
--- subversion/tests/libsvn_fs_base/skel-test.c (revision 11796)
+++ subversion/tests/libsvn_fs_base/skel-test.c (working copy)
@@ -15,6 +15,7 @@
  * ====================================================================
  */
 
+#include <assert.h>
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
@@ -172,12 +173,9 @@
 static void
 put_implicit_length_byte (svn_stringbuf_t *str, char byte, char term)
 {
- if (! skel_is_name (byte))
- abort ();
- if (term != '\0'
- && ! skel_is_space (term)
- && ! skel_is_paren (term))
- abort ();
+ assert (skel_is_name (byte));
+ assert (term == '\0' || skel_is_space (term) || skel_is_paren (term));
+
   svn_stringbuf_appendbytes (str, &byte, 1);
   if (term != '\0')
     svn_stringbuf_appendbytes (str, &term, 1);
@@ -189,8 +187,7 @@
 static int
 check_implicit_length_byte (skel_t *skel, char byte)
 {
- if (! skel_is_name (byte))
- abort ();
+ assert (skel_is_name (byte));
 
   return check_atom (skel, &byte, 1);
 }
@@ -226,10 +223,7 @@
   apr_size_t len;
   char *name = gen_implicit_length_all_chars (&len);
 
- if (term != '\0'
- && ! skel_is_space (term)
- && ! skel_is_paren (term))
- abort ();
+ assert (term == '\0' || skel_is_space (term) || skel_is_paren (term));
 
   svn_stringbuf_appendbytes (str, name, len);
   if (term != '\0')
@@ -309,8 +303,7 @@
   char *buf = malloc (len + 100);
   apr_size_t length_len;
 
- if (! skel_is_space (sep))
- abort ();
+ assert (skel_is_space (sep));
 
   /* Generate the length and separator character. */
   sprintf (buf, "%"APR_SIZE_T_FMT"%c", len, sep);
@@ -467,8 +460,7 @@
 {
   int i;
 
- if (len > 0 && ! skel_is_space (space))
- abort ();
+ assert (len == 0 || skel_is_space (space));
 
   svn_stringbuf_appendcstr (str, "(");
   for (i = 0; i < len; i++)
@@ -483,8 +475,7 @@
 {
   int i;
 
- if (len > 0 && ! skel_is_space (space))
- abort ();
+ assert (len == 0 || skel_is_space (space));
 
   for (i = 0; i < len; i++)
     svn_stringbuf_appendbytes (str, &space, 1);
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 11796)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -16,6 +16,8 @@
  */
 
 
+#include <assert.h>
+
 #include "svn_private_config.h"
 #include "svn_pools.h"
 #include "svn_error.h"
@@ -252,8 +254,7 @@
   const char *full_path;
 
   /* A path relative to nothing? I don't think so. */
- if (path && (! pb))
- abort();
+ assert (!path || pb);
 
   /* Construct the full path of this node. */
   if (pb)
Index: subversion/libsvn_fs_fs/dag.c
===================================================================
--- subversion/libsvn_fs_fs/dag.c (revision 11796)
+++ subversion/libsvn_fs_fs/dag.c (working copy)
@@ -628,10 +628,7 @@
   /* Oh, give me a clone...
      (If they're the same, we haven't cloned the transaction's root
      directory yet.) */
- if (svn_fs_fs__id_eq (root_id, base_root_id))
- {
- abort ();
- }
+ assert (! svn_fs_fs__id_eq (root_id, base_root_id));
 
   /* One way or another, root_id now identifies a cloned root node. */
   SVN_ERR (svn_fs_fs__dag_get_node (root_p, fs, root_id, pool));

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 9 19:38:29 2004

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