[PATCH] (Bite-size) issue #613: "svn_fs__next_key() should publish a size for users"

The following patch

  * adds a define called SVN_FS__MAX_KEY_SIZE to libsvn_fs/key-gen.h
  * makes all callers use it (exception: the unit tests, which
    deliberately use smaller fixed-size buffers)
  * removes a tiny bit of wheel-reinvention in svn_fs__next_key

After applying the patch, Subversion compiles cleanly and passes
"make check" on my system.

-------------------- patch begins --------------------
Index: ./subversion/libsvn_fs/key-gen.c
===================================================================
--- ./subversion/libsvn_fs/key-gen.c
+++ ./subversion/libsvn_fs/key-gen.c Sun Aug 4 17:04:24 2002
@@ -15,6 +15,9 @@
  * ====================================================================
  */
 
+#include <assert.h>
+#include <string.h>
+
 #define APR_WANT_STRFUNC
 #include <apr_want.h>
 #include <stdlib.h>
@@ -166,17 +169,23 @@
         next[i] = c;
     }
 
- /* Do all possible null terminations in advance... */
- next[olen] = '\0';
- next[olen + 1] = '\0';
+ /* The new length is OLEN, plus 1 if there's a carry out of the
+ leftmost digit. */
+ *len = olen + carry;
+
+ /* Ensure that we haven't overrun the (ludicrous) bound on key length.
+ Note that SVN_FS__MAX_KEY_SIZE is a bound on the size *including*
+ the trailing null byte. */
+ assert (*len < SVN_FS__MAX_KEY_SIZE);
+
+ /* Now we know it's safe to add the null terminator. */
+ next[*len] = '\0';
 
- /* ... then handle any leftover carry. */
+ /* Handle any leftover carry. */
   if (carry)
     {
- for (i = (olen - 1); i >= 0; i--)
- next[i + 1] = next[i];
+ memmove (next+1, next, olen);
       next[0] = '1';
- *len = olen + 1;
     }
 }
 
Index: ./subversion/libsvn_fs/key-gen.h
===================================================================
--- ./subversion/libsvn_fs/key-gen.h
+++ ./subversion/libsvn_fs/key-gen.h Sun Aug 4 16:46:12 2002
@@ -25,6 +25,23 @@
 #endif /* __cplusplus */
 
  
+/* The alphanumeric keys passed in and out of svn_fs__next_key
+ are guaranteed never to be longer than this many bytes,
+ *including* the trailing null byte. It is therefore safe
+ to declare a key as "char key[SVN_FS__MAX_KEY_SIZE]".
+ This will be a problem if the number of representations
+ in a filesystem ever exceeds
+ 18217977168218728251394687124089371267338971528174
+ 76066745969754933395997209053270030282678007662838
+ 67331479599455916367452421574456059646801054954062
+ 15017704234999886990788594743994796171248406730973
+ 80736524850563115569208508785942830080999927310762
+ 50733948404739350551934565743979678824151197232629
+ 947748581376,
+ but that's a risk we'll live with for npw. */
+#define SVN_FS__MAX_KEY_SIZE 200
+
+
 /* Return the value of the string of digits at DATA as an ASCII
    decimal number. The string is at most LEN bytes long. The value
    of the number is at most MAX. Set *END to the address of the first
Index: ./subversion/libsvn_fs/bdb/reps-table.c
===================================================================
--- ./subversion/libsvn_fs/bdb/reps-table.c
+++ ./subversion/libsvn_fs/bdb/reps-table.c Sun Aug 4 16:47:36 2002
@@ -132,17 +132,7 @@
   DBT query, result;
   int db_err;
   apr_size_t len;
- char next_key[200]; /* This will be a problem if the number of
- representations in a filesystem ever
- exceeds 1821797716821872825139468712408937
- 126733897152817476066745969754933395997209
- 053270030282678007662838673314795994559163
- 674524215744560596468010549540621501770423
- 499988699078859474399479617124840673097380
- 736524850563115569208508785942830080999927
- 310762507339484047393505519345657439796788
- 24151197232629947748581376. Somebody warn
- my grandchildren. */
+ char next_key[SVN_FS__MAX_KEY_SIZE];
   
   /* ### todo: see issue #409 for why bumping the key as part of this
      trail is problematic. */
Index: ./subversion/libsvn_fs/bdb/txn-table.c
===================================================================
--- ./subversion/libsvn_fs/bdb/txn-table.c
+++ ./subversion/libsvn_fs/bdb/txn-table.c Sun Aug 4 16:48:04 2002
@@ -100,7 +100,7 @@
 {
   DBT query, result;
   apr_size_t len;
- char next_key[200];
+ char next_key[SVN_FS__MAX_KEY_SIZE];
   int db_err;
 
   svn_fs__str_to_dbt (&query, (char *) svn_fs__next_key_key);
Index: ./subversion/libsvn_fs/bdb/copies-table.c
===================================================================
--- ./subversion/libsvn_fs/bdb/copies-table.c
+++ ./subversion/libsvn_fs/bdb/copies-table.c Sun Aug 4 16:50:11 2002
@@ -90,7 +90,7 @@
 {
   DBT query, result;
   apr_size_t len;
- char next_key[200];
+ char next_key[SVN_FS__MAX_KEY_SIZE];
   int db_err;
 
   svn_fs__str_to_dbt (&query, (char *) svn_fs__next_key_key);
Index: ./subversion/libsvn_fs/bdb/strings-table.c
===================================================================
--- ./subversion/libsvn_fs/bdb/strings-table.c
+++ ./subversion/libsvn_fs/bdb/strings-table.c Sun Aug 4 16:49:33 2002
@@ -245,7 +245,7 @@
 get_key_and_bump (svn_fs_t *fs, const char **key, trail_t *trail)
 {
   DBC *cursor;
- char next_key[200];
+ char next_key[SVN_FS__MAX_KEY_SIZE];
   apr_size_t key_len;
   int db_err;
   DBT query;
Index: ./subversion/libsvn_fs/bdb/nodes-table.c
===================================================================
--- ./subversion/libsvn_fs/bdb/nodes-table.c
+++ ./subversion/libsvn_fs/bdb/nodes-table.c Sun Aug 4 16:48:45 2002
@@ -175,7 +175,7 @@
   DBT key, value;
   svn_fs_id_t *id;
   const char *node_id;
- char next_key[200];
+ char next_key[SVN_FS__MAX_KEY_SIZE];
   apr_size_t len;
 
   /* TXN_ID is required! */
--------------------- patch ends ---------------------

------------------ changelog begins ------------------
2002-08-04 Gareth McCaughan <gareth.mccaughan@pobox.com>

        * key-gen.h: new SVN_FS__MAX_KEY_SIZE define, to replace
        the magic number 200 found in various places.

        * key-gen.c (svn_fs__next_key): add an assertion to ensure
        that bound is never exceeded. Minor reorganization of the
        handling of carry-out.

        * copies-table.c (svn_fs__reserve_copy_id): use new
        SVN_FS__MAX_KEY_SIZE instead of magic number.

        * nodes-table.c (svn_fs__new_node_id): use new
        SVN_FS__MAX_KEY_SIZE instead of magic number.

        * reps-table.c (svn_fs__write_new_rep): use new
        SVN_FS__MAX_KEY_SIZE instead of magic number.

        * strings-table.c (get_key_and_bump): use new
        SVN_FS__MAX_KEY_SIZE instead of magic number.

        * txn-table.c (allocate_txn_id): use new
        SVN_FS__MAX_KEY_SIZE instead of magic number.
------------------- changelog ends -------------------

-- 
g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org