[PATCH 0/1] MR3846: winmm: Handle memory allocation failure in MMDRV_InitPerType (cppcheck).
From: Alex Henrie alexhenrie24@gmail.com
--- dlls/winmm/lolvldrv.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/winmm/lolvldrv.c b/dlls/winmm/lolvldrv.c index e664021dc64..7977fcfba5f 100644 --- a/dlls/winmm/lolvldrv.c +++ b/dlls/winmm/lolvldrv.c @@ -374,6 +374,7 @@ static BOOL MMDRV_InitPerType(LPWINE_MM_DRIVER lpDrv, UINT type, UINT wMsg) /* realloc translation table */ mem = llTypes[type].lpMlds ? llTypes[type].lpMlds - 1 : NULL; mem = realloc(mem, sizeof(WINE_MLD) * (llTypes[type].wMaxId + 1)); + if (!mem) return FALSE; llTypes[type].lpMlds = mem + 1;
/* re-build the translation table */
This merge request was approved by Huw Davies.
I've approved this and !3847, but generally these sort of 'fixes' aren't particularly useful.
I've approved this and !3847, but generally these sort of 'fixes' aren't particularly useful.
I wonder if we should establish some sort of policy around this, because I've seen different maintainers take different positions. (And if checking for allocation failure isn't useful, I don't really see why these patches should be accepted regardless; better not to clutter the code I'd think.)
On Fri Sep 15 17:33:31 2023 +0000, Zebediah Figura wrote:
I've approved this and !3847, but generally these sort of 'fixes'
aren't particularly useful. I wonder if we should establish some sort of policy around this, because I've seen different maintainers take different positions. (And if checking for allocation failure isn't useful, I don't really see why these patches should be accepted regardless; better not to clutter the code I'd think.)
Having a formal memory error handling policy could be helpful. For what it's worth, I've only been submitting patches like this if both a static analyzer warns about it and the API has a clear way to handle an out-of-memory error.
By the way, there are several more important patches of mine waiting in the queue. Looking at static analyzer warnings is just a side project while I wait for feedback on the others.
I wonder if we should establish some sort of policy around this, because I've seen different maintainers take different positions. (And if checking for allocation failure isn't useful, I don't really see why these patches should be accepted regardless; better not to clutter the code I'd think.)
There can of course be good reasons to check for allocation failures - it depends on the situation. Further, if new code is written that includes checks then I don't care and that could to some extent be deemed personal choice. What we probably should avoid is MRs that add unnecessary checks to existing code - we all have better things to spend our time on.
I did accept these MRs because I didn't really want to get into a debate about this...
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Huw Davies (＠huw)
-
Zebediah Figura (＠zfigura)