Simon Glover wrote: > Well, one issue with this patch is that Parrot will now segfault if > (s>buflen + addlen) < 0. It doesn't seem possible to actually provoke > this behaviour at the moment, however - string_grow is only called > from one place in string_replace, and the code in string_replace > ensures that addlen > 0. > > It's easy enough to add a test in string_grow to avoid the problem, > but I'm not sure what we should return in that case: the input > string s, or a newly created empty string?
Dan has since rewritten string_grow to use Parrot_reallocate; however, the problem still remains. In this particular case, based on the name, description and usage of the function, there seems to be no reason not to change addlen to be a UINTVAL. However, this raises a more general point regarding preconditions. At one point do we start/stop trusting ourselves? string_replace calls string_grow which calls Parrot_reallocate which calls mem_allocate. If we say that string_replace (or other string_* functions) cannot be trusted to call string_grow correctly, surely we must then modify mem_allocate to handle a negative request length. -- Peter Gibbs EmKel Systems
