This patch tidies up the formatting of string_substr and string_replace,
 adds a bunch of comments, and moves a couple of assignments closer to
 where the variables are actually used; none of the functionality should
 be affected. All tests still pass (including the ones in a previous patch
 that nobody's applied yet)

 Simon


--- string.c.old        Mon Apr 15 21:00:00 2002
+++ string.c    Tue Apr 16 17:00:03 2002
@@ -362,7 +362,6 @@
         return string_make(interpreter, NULL, 0, src->encoding, 0, src->type);
     }

-    true_length = (UINTVAL)length;
     if (offset < 0) {
         true_offset = (UINTVAL)(src->strlen + offset);
     }
@@ -371,18 +370,20 @@
         internal_exception(SUBSTR_OUT_OF_STRING,
                            "Cannot take substr outside string");
     }
+
+    true_length = (UINTVAL)length;
     if (true_length > (src->strlen - true_offset)) {
         true_length = (UINTVAL)(src->strlen - true_offset);
     }

     substart_off = (const char *)src->encoding->skip_forward(src->bufstart,
-                                                       true_offset) -
-        (char *)src->bufstart;
+                                                             true_offset)
+                   - (char *)src->bufstart;
+
     subend_off =
-        (const char *)src->encoding->skip_forward((char *)src->bufstart +
-                                            substart_off,
-                                            true_length) -
-        (char *)src->bufstart;
+        (const char *)src->encoding->skip_forward((char *)src->bufstart
+                                                  + substart_off, true_length)
+         - (char *)src->bufstart;

     dest =
         string_make(interpreter, NULL, true_length * src->encoding->max_bytes,
@@ -428,37 +429,43 @@
     INTVAL diff;

     true_offset = (UINTVAL)offset;
-    true_length = (UINTVAL)length;

-    if(rep->encoding != src->encoding || rep->type != src->type)
-        rep = string_transcode(interpreter, rep, src->encoding, src->type, NULL);
+    /* If the offset is negative, then abs(offset) should not be larger
+     * than the length of the string. If it is, then the cast ensures
+     * that true_offset is larger than any possible string length (and
+     * hence will generate an exception in the next statement).
+     */

-    /* abs(-offset) may not be > strlen-1 */
     if (offset < 0) {
         true_offset = (UINTVAL)(src->strlen + offset);
     }

-    /* Can replace 1 past end of string which is technically outside the string
-     * but is same as a concat().
-     * Only give exception if caller trys to replace end of string + 2
+    /* Replacing one character past the end of the string has the
+     * same effect as string_concat(); therefore, only throw an
+     * exception if the caller tries to replace two or more characters
+     * past the end of the string
      */
     if (true_offset > src->strlen) {
         internal_exception(SUBSTR_OUT_OF_STRING,
                "Can only replace inside string or index after end of string");
     }
+
+    true_length = (UINTVAL)length;
     if (true_length > (src->strlen - true_offset)) {
         true_length = (UINTVAL)(src->strlen - true_offset);
     }

-    /* Save the substring that is replaced for the return value */
-    substart_off = (char *)src->encoding->skip_forward(src->bufstart,
-                                                       true_offset) -
-        (char *)src->bufstart;
+    /* Construct and save the substring that is replaced; this will be
+     * the function return value
+     */
+    substart_off = (const char *)src->encoding->skip_forward(src->bufstart,
+                                                       true_offset)
+                  - (char *)src->bufstart;
+
     subend_off =
-        (char *)src->encoding->skip_forward((char *)src->bufstart +
-                                            substart_off,
-                                            true_length) -
-        (char *)src->bufstart;
+        (const char *)src->encoding->skip_forward((char *)src->bufstart
+                                                  + substart_off, true_length)
+        - (char *)src->bufstart;

     if (subend_off < substart_off) {
         internal_exception(SUBSTR_OUT_OF_STRING,
@@ -479,43 +486,54 @@
     }

     /* Now do the replacement */
+
+    if(rep->encoding != src->encoding || rep->type != src->type)
+        rep =
+           string_transcode(interpreter, rep, src->encoding, src->type, NULL);

     /*
-     * If the replacement string fits inside the original substring
+     * If the replacement string fits inside the original substring,
+     * or there's sufficient space left in the string buffer, then
      * don't create a new string, just pack it.
      */
+
     diff = (subend_off - substart_off) - rep->bufused;

     if(diff >= 0
         || ((INTVAL)src->bufused - (INTVAL)src->buflen) <= diff) {
-
+
         if(diff != 0) {
             mem_sys_memmove((char*)src->bufstart + substart_off + rep->bufused,
-                                (char*)src->bufstart + subend_off,
-                                src->buflen - (subend_off - diff));
+                            (char*)src->bufstart + subend_off,
+                            src->buflen - (subend_off - diff));
+
             src->bufused -= diff;
         }

         mem_sys_memcopy((char*)src->bufstart + substart_off,
-                                rep->bufstart, rep->bufused);
+                               rep->bufstart, rep->bufused);
+
         if(diff != 0)
             (void)string_compute_strlen(src);
     }
     /*
-     * Replacement is larger than avail buffer, grow the string
+     * The replacement is larger than the space available, so grow the string
      */
     else {
         /* diff is negative here, make it positive */
         diff = -(diff);
+
         string_grow(interpreter, src, diff);

-        /* Move the end of old string that isn't replaced to new offset first */
+        /* Move the end of the old string that isn't replaced to its new
+         * offset first */
         mem_sys_memmove((char*)src->bufstart + subend_off + diff,
-                                (char*)src->bufstart + subend_off,
-                                src->buflen - subend_off);
+                        (char*)src->bufstart + subend_off,
+                        src->buflen - subend_off);
+
         /* Copy the replacement in */
-        mem_sys_memcopy((char *)src->bufstart + substart_off, rep->bufstart,
-                                rep->bufused);
+        mem_sys_memcopy((char *)src->bufstart + substart_off,
+                                rep->bufstart, rep->bufused);
         src->bufused += diff;
         (void)string_compute_strlen(src);
     }

Reply via email to