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); }