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

Re: [PATCH] add pool pointer to svn_string_t

From: Joe Orton <joe_at_light.plus.com>
Date: 2000-10-11 22:10:35 CEST

On Tue, Oct 10, 2000 at 06:39:10PM -0400, Greg Hudson wrote:
> I support this change, but it might be nice to hear the opion of one
> of the original Subversion people for such a basic change.

Yes, I am not going to commit this without approval.

> > + /* pool from which the string was originally allocated -- used only
> > + for growing the string, and is NOT destroyed when the string is
> > + destroyed */
>
> You can't destroy a Subversion string, so this comment doesn't quite
> make sense. I'm not sure what alternative wording to use; perhaps
> "and is NOT necessarily specific to the string".

Okay, will change...

> > This also removes the feature that svn_string_append* will allocate
> > the string if they are passed the NULL.
>
> That feature doesn't make sense anyway. (A string is allocated and
> then immediately forgotten about.) I think someone wasn't paying
> attention when they put that in.

I did think it was a silly feature, didn't realise it was *that* silly
though. :)

Updated patch: [sorry these patches are quite big]
- Changes comment as suggested
- Covers new base64.c

Index: include/svn_string.h
===================================================================
RCS file: /cvs/subversion/subversion/include/svn_string.h,v
retrieving revision 1.35
diff -u -r1.35 svn_string.h
--- include/svn_string.h 2000/10/02 18:29:04 1.35
+++ include/svn_string.h 2000/10/11 20:08:59
@@ -63,6 +63,10 @@
   char *data; /* pointer to the bytestring */
   apr_size_t len; /* length of bytestring */
   apr_size_t blocksize; /* total size of buffer allocated */
+ /* pool from which this string was originally allocated, and is not
+ necessarily specific to this string. This is used only for
+ allocating more memory from when the string needs to grow. */
+ apr_pool_t *pool;
 } svn_string_t;
 
 
@@ -77,11 +81,9 @@
 
 /* Make sure that the string STR has at least MINIMUM_SIZE bytes of
    space available in the memory block. (MINIMUM_SIZE should include
- space for the terminating null character.) If we need more memory,
- get it from POOL. */
+ space for the terminating null character.) */
 void svn_string_ensure (svn_string_t *str,
- apr_size_t minimum_size,
- apr_pool_t *pool);
+ apr_size_t minimum_size);
 
 /* Set/get a bytestring empty */
 
@@ -97,14 +99,11 @@
    onto a svn_string_t. reallocs() if necessary. */
 
 void svn_string_appendbytes (svn_string_t *str, const char *bytes,
- const size_t count, apr_pool_t *pool);
+ const size_t count);
 void svn_string_appendstr (svn_string_t *targetstr,
- const svn_string_t *appendstr,
- apr_pool_t *pool);
+ const svn_string_t *appendstr);
 void svn_string_appendcstr (svn_string_t *targetstr,
- const char *cstr,
- apr_pool_t *pool);
-
+ const char *cstr);
 
 /* Duplicate a bytestring; returns freshly malloc'd copy. */
 
Index: libsvn_delta/svndiff.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_delta/svndiff.c,v
retrieving revision 1.5
diff -u -r1.5 svndiff.c
--- libsvn_delta/svndiff.c 2000/10/11 01:19:10 1.5
+++ libsvn_delta/svndiff.c 2000/10/11 20:09:00
@@ -125,7 +125,7 @@
   char buf[128], *p;
 
   p = encode_int (buf, val);
- svn_string_appendbytes (header, buf, p - buf, pool);
+ svn_string_appendbytes (header, buf, p - buf);
 }
 
 
@@ -177,7 +177,7 @@
         ip = encode_int (ip + 1, op->length);
       if (op->action_code != svn_txdelta_new)
         ip = encode_int (ip, op->offset);
- svn_string_appendbytes (instructions, ibuf, ip - ibuf, pool);
+ svn_string_appendbytes (instructions, ibuf, ip - ibuf);
     }
 
   /* Encode the header. */
@@ -409,7 +409,7 @@
     }
 
   /* Concatenate the old with the new. */
- svn_string_appendbytes (db->buffer, buffer, *len, db->subpool);
+ svn_string_appendbytes (db->buffer, buffer, *len);
 
   /* Read the header, if we have enough bytes for that. */
   p = (const unsigned char *) db->buffer->data;
Index: libsvn_delta/text_delta.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_delta/text_delta.c,v
retrieving revision 1.15
diff -u -r1.15 text_delta.c
--- libsvn_delta/text_delta.c 2000/10/10 07:27:50 1.15
+++ libsvn_delta/text_delta.c 2000/10/11 20:09:00
@@ -163,7 +163,7 @@
       op->action_code = opcode;
       op->offset = window->new->len;
       op->length = length;
- svn_string_appendbytes (window->new, new_data, length, window->pool);
+ svn_string_appendbytes (window->new, new_data, length);
       break;
     default:
       assert (!"unknown delta op.");
Index: libsvn_delta/xml_parse.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_delta/xml_parse.c,v
retrieving revision 1.122
diff -u -r1.122 xml_parse.c
--- libsvn_delta/xml_parse.c 2000/10/10 07:27:50 1.122
+++ libsvn_delta/xml_parse.c 2000/10/11 20:09:03
@@ -1300,8 +1300,7 @@
 
       if (digger->current_propdelta)
         svn_string_appendbytes (digger->current_propdelta->value,
- data, length,
- digger->pool);
+ data, length);
     }
 
   else
Index: libsvn_fs/id.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/id.c,v
retrieving revision 1.3
diff -u -r1.3 id.c
--- libsvn_fs/id.c 2000/10/11 18:18:29 1.3
+++ libsvn_fs/id.c 2000/10/11 20:09:04
@@ -294,7 +294,7 @@
       if (id[i + 1] != -1)
         buf[len++] = '.';
 
- svn_string_appendbytes (unparsed, buf, len, pool);
+ svn_string_appendbytes (unparsed, buf, len);
     }
 
   return unparsed;
Index: libsvn_fs/skel.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/skel.c,v
retrieving revision 1.4
diff -u -r1.4 skel.c
--- libsvn_fs/skel.c 2000/10/11 18:18:29 1.4
+++ libsvn_fs/skel.c 2000/10/11 20:09:05
@@ -395,8 +395,8 @@
       /* Append an atom to STR. */
       if (use_implicit (skel))
         {
- svn_string_appendbytes (str, skel->data, skel->len, pool);
- svn_string_appendbytes (str, " ", 1, pool);
+ svn_string_appendbytes (str, skel->data, skel->len);
+ svn_string_appendbytes (str, " ", 1);
         }
       else
         {
@@ -411,11 +411,10 @@
           /* Make sure we have room for the length, the space, and the
              atom's contents. */
           svn_string_ensure (str,
- str->len + length_len + 1 + skel->len,
- pool);
- svn_string_appendbytes (str, buf, length_len, pool);
+ str->len + length_len + 1 + skel->len);
+ svn_string_appendbytes (str, buf, length_len);
           str->data[str->len++] = '\n';
- svn_string_appendbytes (str, skel->data, skel->len, pool);
+ svn_string_appendbytes (str, skel->data, skel->len);
         }
     }
   else
@@ -425,7 +424,7 @@
       int i;
 
       /* The opening paren has been indented by the parent, if necessary. */
- svn_string_ensure (str, str->len + 1, pool);
+ svn_string_ensure (str, str->len + 1);
       str->data[str->len++] = '(';
       
       depth++;
@@ -434,7 +433,7 @@
       for (child = skel->children; child; child = child->next)
         {
           /* Add a newline, and indentation. */
- svn_string_ensure (str, str->len + 1 + depth * 2, pool);
+ svn_string_ensure (str, str->len + 1 + depth * 2);
           str->data[str->len++] = '\n';
           for (i = 0; i < depth * 2; i++)
             str->data[str->len++] = ' ';
@@ -448,7 +447,7 @@
          There should be no newline after a closing paren; a skel must
          entirely fill its string. If we're part of a parent list,
          the parent will take care of adding that. */
- svn_string_ensure (str, str->len + 1 + depth * 2 + 1, pool);
+ svn_string_ensure (str, str->len + 1 + depth * 2 + 1);
       str->data[str->len++] = '\n';
       for (i = 0; i < depth * 2; i++)
         str->data[str->len++] = ' ';
Index: libsvn_string/svn_string.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_string/svn_string.c,v
retrieving revision 1.49
diff -u -r1.49 svn_string.c
--- libsvn_string/svn_string.c 2000/10/03 22:33:37 1.49
+++ libsvn_string/svn_string.c 2000/10/11 20:09:06
@@ -97,6 +97,7 @@
   new_string->data = (char *) apr_palloc (pool, size + 1);
   new_string->len = size;
   new_string->blocksize = size + 1;
+ new_string->pool = pool;
 
   memcpy (new_string->data, bytes, size);
 
@@ -161,8 +162,7 @@
 
 void
 svn_string_ensure (svn_string_t *str,
- apr_size_t minimum_size,
- apr_pool_t *pool)
+ apr_size_t minimum_size)
 {
   /* Keep doubling capacity until have enough. */
   if (str->blocksize < minimum_size)
@@ -177,28 +177,22 @@
   str->data = (char *) my__realloc (str->data,
                                     str->len,
                                     str->blocksize,
- pool);
+ str->pool);
 }
 
 
 /* Copy COUNT bytes from BYTES onto the end of bytestring STR. */
 void
 svn_string_appendbytes (svn_string_t *str, const char *bytes,
- const apr_size_t count, apr_pool_t *pool)
+ const apr_size_t count)
 {
   apr_size_t total_len;
   void *start_address;
 
- if (str == NULL)
- {
- str = svn_string_ncreate (bytes, count, pool);
- return;
- }
-
   total_len = str->len + count; /* total size needed */
 
   /* +1 for null terminator. */
- svn_string_ensure (str, (total_len + 1), pool);
+ svn_string_ensure (str, (total_len + 1));
 
   /* get address 1 byte beyond end of original bytestring */
   start_address = (str->data + str->len);
@@ -214,21 +208,17 @@
 
 /* Append APPENDSTR onto TARGETSTR. */
 void
-svn_string_appendstr (svn_string_t *targetstr, const svn_string_t *appendstr,
- apr_pool_t *pool)
+svn_string_appendstr (svn_string_t *targetstr, const svn_string_t *appendstr)
 {
- svn_string_appendbytes (targetstr, appendstr->data,
- appendstr->len, pool);
+ svn_string_appendbytes (targetstr, appendstr->data, appendstr->len);
 }
 
 
 /* Append CSTR onto TARGETSTR. */
 void
-svn_string_appendcstr (svn_string_t *targetstr,
- const char *cstr,
- apr_pool_t *pool)
+svn_string_appendcstr (svn_string_t *targetstr, const char *cstr)
 {
- svn_string_appendbytes (targetstr, cstr, strlen(cstr), pool);
+ svn_string_appendbytes (targetstr, cstr, strlen(cstr));
 }
 
 
Index: libsvn_subr/base64.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_subr/base64.c,v
retrieving revision 1.1
diff -u -r1.1 base64.c
--- libsvn_subr/base64.c 2000/10/11 19:19:51 1.1
+++ libsvn_subr/base64.c 2000/10/11 20:09:06
@@ -112,11 +112,11 @@
           memset (eb->buf + eb->buflen, 0, 3 - eb->buflen);
           encode_group (eb->buf, group);
           memset (group + (eb->buflen + 1), '=', 4 - (eb->buflen + 1));
- svn_string_appendbytes (encoded, group, 4, subpool);
+ svn_string_appendbytes (encoded, group, 4);
           eb->linelen += 4;
         }
       if (eb->linelen != 0)
- svn_string_appendcstr (encoded, "\n", subpool);
+ svn_string_appendcstr (encoded, "\n");
     }
   else
     {
@@ -129,12 +129,12 @@
           memcpy (eb->buf + eb->buflen, p, 3 - eb->buflen);
           p += (3 - eb->buflen);
           encode_group (eb->buf, group);
- svn_string_appendbytes (encoded, group, 4, subpool);
+ svn_string_appendbytes (encoded, group, 4);
           eb->buflen = 0;
           eb->linelen += 4;
           if (eb->linelen == BASE64_LINELEN)
             {
- svn_string_appendcstr (encoded, "\n", subpool);
+ svn_string_appendcstr (encoded, "\n");
               eb->linelen = 0;
             }
         }
@@ -238,7 +238,7 @@
             {
               memset (db->buf + db->buflen, 0, 4 - db->buflen);
               decode_group (db->buf, group);
- svn_string_appendbytes (decoded, group, db->buflen - 1, subpool);
+ svn_string_appendbytes (decoded, group, db->buflen - 1);
             }
           db->done = TRUE;
         }
@@ -250,7 +250,7 @@
           if (db->buflen == 4)
             {
               decode_group (db->buf, group);
- svn_string_appendbytes (decoded, group, 3, subpool);
+ svn_string_appendbytes (decoded, group, 3);
               db->buflen = 0;
             }
         }
Index: libsvn_subr/path.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_subr/path.c,v
retrieving revision 1.13
diff -u -r1.13 path.c
--- libsvn_subr/path.c 2000/10/04 20:50:20 1.13
+++ libsvn_subr/path.c 2000/10/11 20:09:06
@@ -88,9 +88,9 @@
   char dirsep = SVN_PATH_REPOS_SEPARATOR;
 
   if (! svn_string_isempty (path))
- svn_string_appendbytes (path, &dirsep, sizeof (dirsep), pool);
+ svn_string_appendbytes (path, &dirsep, sizeof (dirsep));
 
- svn_string_appendbytes (path, component, len, pool);
+ svn_string_appendbytes (path, component, len);
   canonicalize (path, style);
 }
 
Index: libsvn_subr/svn_parse.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_subr/svn_parse.c,v
retrieving revision 1.39
diff -u -r1.39 svn_parse.c
--- libsvn_subr/svn_parse.c 2000/10/10 00:28:25 1.39
+++ libsvn_subr/svn_parse.c 2000/10/11 20:09:07
@@ -85,14 +85,14 @@
       if (c == '\n') /* line is finished. */
         {
           /* store the newline in our bytestring (important!) */
- svn_string_appendbytes (line, &c, 1, pool);
+ svn_string_appendbytes (line, &c, 1);
 
           return APR_SUCCESS;
         }
       
       else /* otherwise, just append this byte to the bytestring */
         {
- svn_string_appendbytes (line, &c, 1, pool);
+ svn_string_appendbytes (line, &c, 1);
         }
     }
 }
@@ -135,8 +135,7 @@
 
           svn_string_appendbytes (*substr, /* new substring */
                                   searchstr->data + start,/* start copy */
- (i - start), /* number to copy */
- pool);
+ (i - start)); /* number to copy */
           
           svn_string_strip_whitespace (*substr);
 
@@ -200,7 +199,7 @@
       char *finalmsg; /* TODO: use apr's sprintf() thingie here */
       svn_string_t *msg = svn_string_create
         ("svn_parse(): can't open for reading, file ", pool);
- svn_string_appendstr (msg, filename, pool);
+ svn_string_appendstr (msg, filename);
 
       return svn_error_create (result, NULL, NULL, pool, msg->data);
     }
Index: libsvn_subr/xml.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_subr/xml.c,v
retrieving revision 1.26
diff -u -r1.26 xml.c
--- libsvn_subr/xml.c 2000/10/10 00:28:25 1.26
+++ libsvn_subr/xml.c 2000/10/11 20:09:08
@@ -81,7 +81,7 @@
       while (q < end && *q != '&' && *q != '<' && *q != '>'
              && *q != '"' && *q != '\'')
         q++;
- svn_string_appendbytes (*outstr, p, q - p, pool);
+ svn_string_appendbytes (*outstr, p, q - p);
 
       /* We may already be a winner. */
       if (q == end)
@@ -89,15 +89,15 @@
 
       /* Append the entity reference for the character. */
       if (*q == '&')
- svn_string_appendcstr (*outstr, "&", pool);
+ svn_string_appendcstr (*outstr, "&");
       else if (*q == '<')
- svn_string_appendcstr (*outstr, "<", pool);
+ svn_string_appendcstr (*outstr, "<");
       else if (*q == '>')
- svn_string_appendcstr (*outstr, ">", pool);
+ svn_string_appendcstr (*outstr, ">");
       else if (*q == '"')
- svn_string_appendcstr (*outstr, """, pool);
+ svn_string_appendcstr (*outstr, """);
       else if (*q == '\'')
- svn_string_appendcstr (*outstr, "'", pool);
+ svn_string_appendcstr (*outstr, "'");
 
       p = q + 1;
     }
@@ -238,8 +238,7 @@
 {
   if (*str == NULL)
     *str = svn_string_create ("", pool);
- svn_string_appendcstr (*str, "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n",
- pool);
+ svn_string_appendcstr (*str, "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n");
 }
 
 
@@ -336,8 +335,8 @@
   if (*str == NULL)
     *str = svn_string_create ("", pool);
 
- svn_string_appendcstr (*str, "<", pool);
- svn_string_appendcstr (*str, tagname, pool);
+ svn_string_appendcstr (*str, "<");
+ svn_string_appendcstr (*str, tagname);
 
   for (hi = apr_hash_first (attributes); hi; hi = apr_hash_next (hi))
     {
@@ -348,18 +347,18 @@
       apr_hash_this (hi, &key, &keylen, &val);
       assert (val != NULL);
 
- svn_string_appendcstr (*str, "\n ", pool);
- svn_string_appendcstr (*str, (char *) key, pool);
- svn_string_appendcstr (*str, "=\"", pool);
+ svn_string_appendcstr (*str, "\n ");
+ svn_string_appendcstr (*str, (char *) key);
+ svn_string_appendcstr (*str, "=\"");
       svn_xml_escape_string (str, (svn_string_t *) val, pool);
- svn_string_appendcstr (*str, "\"", pool);
+ svn_string_appendcstr (*str, "\"");
     }
 
   if (style == svn_xml_self_closing)
- svn_string_appendcstr (*str, "/", pool);
- svn_string_appendcstr (*str, ">", pool);
+ svn_string_appendcstr (*str, "/");
+ svn_string_appendcstr (*str, ">");
   if (style != svn_xml_protect_pcdata)
- svn_string_appendcstr (*str, "\n", pool);
+ svn_string_appendcstr (*str, "\n");
 }
 
 
@@ -401,9 +400,9 @@
   if (*str == NULL)
     *str = svn_string_create ("", pool);
 
- svn_string_appendcstr (*str, "</", pool);
- svn_string_appendcstr (*str, tagname, pool);
- svn_string_appendcstr (*str, ">\n", pool);
+ svn_string_appendcstr (*str, "</");
+ svn_string_appendcstr (*str, tagname);
+ svn_string_appendcstr (*str, ">\n");
 }
 
 
Index: libsvn_fs/tests/skel-test.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/tests/skel-test.c,v
retrieving revision 1.4
diff -u -r1.4 skel-test.c
--- libsvn_fs/tests/skel-test.c 2000/10/11 18:19:43 1.4
+++ libsvn_fs/tests/skel-test.c 2000/10/11 20:09:09
@@ -178,8 +178,8 @@
     abort ();
   if (! skel_is_space (space))
     abort ();
- svn_string_appendbytes (str, &byte, 1, pool);
- svn_string_appendbytes (str, &space, 1, pool);
+ svn_string_appendbytes (str, &byte, 1);
+ svn_string_appendbytes (str, &space, 1);
 }
 
 
@@ -229,8 +229,8 @@
   if (! skel_is_space (space))
     abort ();
 
- svn_string_appendbytes (str, name, len, pool);
- svn_string_appendbytes (str, &space, 1, pool);
+ svn_string_appendbytes (str, name, len);
+ svn_string_appendbytes (str, &space, 1);
 }
 
 
@@ -313,7 +313,7 @@
   /* Copy in the real data (which may contain nulls). */
   memcpy (buf + length_len, data, len);
 
- svn_string_appendbytes (str, buf, length_len + len, pool);
+ svn_string_appendbytes (str, buf, length_len + len);
 }
 
 
@@ -409,9 +409,9 @@
   if (len > 0 && ! skel_is_space (space))
     abort ();
 
- svn_string_appendcstr (str, "(", pool);
+ svn_string_appendcstr (str, "(");
   for (i = 0; i < len; i++)
- svn_string_appendbytes (str, &space, 1, pool);
+ svn_string_appendbytes (str, &space, 1);
 }
 
 
@@ -426,8 +426,8 @@
     abort ();
 
   for (i = 0; i < len; i++)
- svn_string_appendbytes (str, &space, 1, pool);
- svn_string_appendcstr (str, ")", pool);
+ svn_string_appendbytes (str, &space, 1);
+ svn_string_appendcstr (str, ")");
 }
 
 
@@ -621,7 +621,7 @@
               /* A list containing an invalid element. */
               str = get_empty_string ();
               put_list_start (str, sep, sep_count);
- svn_string_appendcstr (str, "100 ", pool);
+ svn_string_appendcstr (str, "100 ");
               put_list_end (str, sep, sep_count);
               if (parse_str (str))
                 return fail ();
Index: libsvn_string/tests/stringtest.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_string/tests/stringtest.c,v
retrieving revision 1.27
diff -u -r1.27 stringtest.c
--- libsvn_string/tests/stringtest.c 2000/09/21 22:18:28 1.27
+++ libsvn_string/tests/stringtest.c 2000/10/11 20:09:10
@@ -114,7 +114,7 @@
   strcpy (tmp, a->data);
   strcat (tmp, b->data);
   old_len = a->len;
- svn_string_appendstr (a, b, pool);
+ svn_string_appendstr (a, b);
   
   /* Test that length, data, and null-termination are correct. */
   if ((a->len == (old_len + b->len)) && ((strcmp (a->data, tmp)) == 0))
@@ -128,7 +128,7 @@
 test4 (const char **msg)
 {
   a = svn_string_create (phrase_1, pool);
- svn_string_appendcstr (a, "new bytes to append", pool);
+ svn_string_appendcstr (a, "new bytes to append");
   
   *msg = "append C string to svn_string_t";
 
@@ -145,7 +145,7 @@
 test5 (const char **msg)
 {
   a = svn_string_create (phrase_1, pool);
- svn_string_appendbytes (a, "new bytes to append", 9, pool);
+ svn_string_appendbytes (a, "new bytes to append", 9);
   
   *msg = "append bytes, then compare two strings";
 
@@ -289,7 +289,7 @@
   block_len_1 = (s->blocksize);
   
   t = svn_string_create (", plus a string more than twice as long", pool);
- svn_string_appendstr (s, t, pool);
+ svn_string_appendstr (s, t);
   len_2 = (s->len);
   block_len_2 = (s->blocksize);
Received on Sat Oct 21 14:36:10 2006

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.