Don't crash if more than 128 modes are available

[smcv]

No description provided.

[ensiform]

I would suggest a more informative error message when out of memory. Simply saying out of memory is pretty basic and not helpful for users.

[timangus]

The chances of anyone ever seeing that message are extremely slim. Don't worry about it.

[smcv]

On normal Linux systems (and probably most other Unix flavours), malloc never actually returns NULL, because of overcommit behaviour. It can be configured to avoid overcommitting, but that's very unusual. Instead, the kernel optimistically provides some address space in the hope that, when it becomes necessary, it can back it with memory. Later, if it turns out the kernel needs more memory than is actually available, a heuristic (the OOM killer) tries to identify which process is misbehaving most badly and shoots it in the head (equivalent to kill -9).

In practice, OOM code paths are basically never tested, which usually means they don't work so making them more complicated than absolutely necessary is wasteful.

In any case, if we don't have enough memory for the number of video modes (if you get more than 128 in practice I'd be amazed) * the size of a SDL rectangle, then we've already lost; that isn't actually very much memory.

Comparing with the behaviour on equally low memory without this patch, even when we have < 128 modes: on entry to the function, we adjust the stack pointer to give us enough memory for 128 video modes, then try using it. If there isn't actually enough memory to back that array, the kernel can't satisfy our request to write to those pages, so it will either kill us with SIGSEGV, or invoke the OOM killer to find a suitable victim to shoot in the head so that we can continue. This is not actually any more graceful :-)

[smcv]

Any more thoughts on this?

I am currently applying this in Debian's ioquake3 package, because it seems wrong to ship code with a known stack buffer overflow, even one that is highly unlikely to appear in practice (> 128 modes).

If ioquake3 maintainers would prefer, I could change the patch so that instead of allocating short-term dynamic memory (which could in principle fail, although in practice if it ever does we're already doomed), it just ignores any modes beyond the 128th?