add mutt_mem_recalloc()

[flatcap]

Add a function to resize a block of memory.
If the size increases, then zero the new bit.

Discussion

There are about 90 calls to mutt_mem_realloc() in the code.
They fall into four categories:

• Structured ('n' repeated blocks)
  A dynamic array of objects / pointers

• Unstructured (one large undifferentiated block)

• Wipe
  The new memory is memset(0)d, or zeroed with a for loop

• No wipe
  New memory is left uninitialised

Should we always zero the new block?

For a small cost, should we initialise all memory?
Even if it's going to get overwritten soon?

Should we replace any malloc()s with calloc() and recalloc()?

Is recalloc() a worthwhile step forward?

I worked through a handful of functions which dynamically allocate arrays.
They would be tidier using recalloc(), but ultimately they'd probably be better off using ARRAY.
Upgrading to ARRAY would be quite a bit of work.

Add recalloc()

Ages ago, on IRC, we discussed a recalloc() function that would streamline realloc()/memset()

Here's what it could look like...

neomutt/mutt/memory.c

Line 153 in a7f7cf1

void mutt_mem_recalloc(void *ptr, size_t cur_size, size_t new_size)

Before recalloc()

• Calculate the new size
• Reallocate the memory
• Wipe the new bit

neomutt/editor/state.c

Lines 62 to 67 in 371a8ad

	num = ROUND_UP(num + 4, 128);
	mutt_mem_realloc(&es->wbuf, num * sizeof(wchar_t));
	memset(es->wbuf + es->wbuflen, 0, (num - es->wbuflen) * sizeof(wchar_t));
	es->wbuflen = num;

After recalloc()

neomutt/editor/state.c

Lines 61 to 66 in becd9a0

	num = ROUND_UP(num + 4, 128);
	const size_t cur_size = es->wbuflen * sizeof(wchar_t);
	const size_t new_size = num * sizeof(wchar_t);
	mutt_mem_recalloc(&es->wbuf, cur_size, new_size);
	es->wbuflen = num;

The code usually stores the number of items contained in the array,
so for tidiness I've done the calculations first.

It's a bit better, but still a bit clunky.

Add recallocarray()

neomutt/mutt/memory.h

Line 57 in a7f7cf1

mutt_mem_recalloc(ptr, cur_num * block_size, new_num * block_size);

This is just a convenience wrapper around recalloc() -- it simplifies the maths.

After recallocarray()

neomutt/editor/state.c

Lines 61 to 64 in a7f7cf1

	num = ROUND_UP(num + 4, 128);
	mutt_mem_recallocarray(&es->wbuf, es->wbuflen, num, sizeof(wchar_t));
	es->wbuflen = num;

[vuori]

I think this is a good idea. Zeroing memory will either prevent UAFs (null pointer is detected) or cause a more consistent and easier to debug crash.

[flatcap]

Update: Renamed functions -- Prior art in BSD.
Thanks, @alejandro-colomar.

[alejandro-colomar]

Update: Renamed functions -- Prior art in BSD. Thanks, @alejandro-colomar.

Ahhh, I didn't find this PR! :-)

[alejandro-colomar]

Hi @flatcap !

Sorry; I just remembered a detail about recallocarray(3): not only it clears new memory, it also clears the freed memory (like explicit_bzero(3)).

Maybe we should also clear the freed space if new_size < cur_size?

Or if we don't care about that, use a slightly different name such as rezalloc()?

[flatcap]

Maybe we should also clear the freed space if new_size < cur_size?

Hmm... I hadn't considered that.
I can't actually think of any cases that we shrink an array; they tend to get nuked and rebuilt.
Probably worth doing.