Re: Bug#245678: zsh: built-in rm -rf fills up the memory

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Mon, 10 May 2004, Peter Stephenson wrote:

> This is all ghastly even by zsh standards.

Well, that's what comes of having glued MMAP into the hrealloc() algorithm 
piecemeal.  Probably what should have been done was just to #ifdef the 
entire function body.

> What we should do is the equivalent of realloc as an #else of the code
> above --- mmap a larger chunk, unmap the old one, and return the new
> one.

I think I said that, though I hadn't got so far as to pinpoint where it
needed to happen.

> However, I don't think even *that* is the basic problem.  I think this:
> 
>     old = (old + H_ISIZE - 1) & ~(H_ISIZE - 1);
>     new = (new + H_ISIZE - 1) & ~(H_ISIZE - 1);
> 
> is wrong --- it should be the heap size here, not H_ISIZE which is
> simply sizeof(union mem_align), which is probably only 4 or 8 words.

It took me a while staring at this to decide that it was doing the right 
thing, but I came to the conclusion that it's only actually _wrong_ if
HEAP_ARENA_SIZE is _not_ a multiple of H_ISIZE.  And even then I'm pretty
sure "wrong" just means "inefficient".

> We only need to reallocate to heap size boundaries

That's correct, but we _do_ only reallocate to heap size boundaries in the
non-MMAP case.  I think.  It's only the MMAP'd code, which as I said was
pasted in long after the original algorithm was written, that botches it.

> I might be tempted by proposed solutions involving the complete removal
> of hrealloc.

Which would mean what, exactly?  Never grow heap blocks?

(Note that I didn't begin with "why".)