Re: [PATHC] Fix handling of reversed slices in assignstrvalue

>
> Also, now that I look at it more, I'm confused what the purpose of the
> first hunk is? How does allowing the second index to run off the end
> of the string help us here?


The first hunk is dead code. Here is the full context:

if (v->end < 0) {
  // => v->end < 0
  v->end += zlen;
  // => v->end < zlen
  if (v->end < 0) {
    v->end = 0;
  } else if (v->end >= zlen) { // => never true
    v->end = zlen;
  } else {
#ifdef MULTIBYTE_SUPPORT
    if (isset(MULTIBYTE)) {
      // => v->end < zlen
      v->end += MB_METACHARLEN(z + v->end);
      // Here v->end could become greater than zlen if MB_METACHARLEN(...)
returns a value greater than 1
    } else {
      v->end++;
    }
#else
    v->end++;
#endif
  }
}
else if (v->end > zlen)
  v->end = zlen;

One thing that stands out when reviewing it is that the second hunk is
> adding an "if" between an existing "if" and "else if", which feels a
> little weird.


Because of the last comment in the code above, I find it's better to always
check for v->end > zlen. It makes it much more obvious that v->end will
never be greater than zlen at the end of the chunk of code. I assume (but I
haven't checked) that MB_METACHARLEN() never leads v-end to overtake zlen.
So technically keeping the else if would also work and would sometimes
avoid one extra check. If you feel strongly about it, I can re-add it.

Philippe