On Tue, 28 Apr 2026 at 17:21, Lucas Cardoso <[email protected]> wrote: > > In accordance with QEMU's coding style guidelines, replace raw memory > allocation functions (malloc/free) with their GLib equivalents > (g_new/g_malloc0/g_free/g_clear_pointer). > > Also removes the old generic error-handling code from > OPLOpenTable(), since the Glib functions abort the program automatically > if memory is exhausted. > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/1798 > Reviewed-by: Marc-André Lureau <[email protected]> > Signed-off-by: Lucas Cardoso <[email protected]> > --- > v2: > - After some studying, I decided to follow Marc-André's feedback to use > g_clear_pointer() instead of g_free(), on the OPLCloseTable function. > hw/audio/fmopl.c | 43 ++++++++++++------------------------------- > 1 file changed, 12 insertions(+), 31 deletions(-)
I was just reminded by traffic on the issue tracker that we had a discussion on this patch and some debate about different ways to approach it, but never came to a conclusion. My preference is to do what this patch does, i.e. replace the mallocs with g_new(). I think it's a straightforward obviously-correct update to a pretty unloved piece of code, and I'd rather we do that than change to using static arrays. (If anybody does have an interest in modernizing fmopl.c more generally I have no objection; I'd just rather not hang up this bit of general cross-codebase cleanup to stop using raw malloc() that we're nearly done with.) So: Reviewed-by: Peter Maydell <[email protected]> MJT: are you OK with picking it up for -trivial ? > > diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c > index a63ad0f04d..91fe806fd6 100644 > --- a/hw/audio/fmopl.c > +++ b/hw/audio/fmopl.c > @@ -607,26 +607,10 @@ static int OPLOpenTable( void ) > double pom; > > /* allocate dynamic tables */ > - if( (TL_TABLE = malloc(TL_MAX*2*sizeof(int32_t))) == NULL) > - return 0; > - if( (SIN_TABLE = malloc(SIN_ENT*4 *sizeof(int32_t *))) == NULL) > - { > - free(TL_TABLE); > - return 0; > - } > - if( (AMS_TABLE = malloc(AMS_ENT*2 *sizeof(int32_t))) == NULL) > - { > - free(TL_TABLE); > - free(SIN_TABLE); > - return 0; > - } > - if( (VIB_TABLE = malloc(VIB_ENT*2 *sizeof(int32_t))) == NULL) > - { > - free(TL_TABLE); > - free(SIN_TABLE); > - free(AMS_TABLE); > - return 0; > - } > + TL_TABLE = g_new(int32_t, TL_MAX * 2); > + SIN_TABLE = g_new(int32_t *, SIN_ENT * 4); > + AMS_TABLE = g_new(int32_t, AMS_ENT * 2); > + VIB_TABLE = g_new(int32_t, VIB_ENT * 2); > ENV_CURVE = g_new(int32_t, 2 * EG_ENT + 1); > /* make total level table */ > for (t = 0;t < EG_ENT-1 ;t++){ > @@ -695,11 +679,11 @@ static int OPLOpenTable( void ) > > static void OPLCloseTable( void ) > { > - g_free(ENV_CURVE); > - free(TL_TABLE); > - free(SIN_TABLE); > - free(AMS_TABLE); > - free(VIB_TABLE); > + g_clear_pointer(&ENV_CURVE, g_free); > + g_clear_pointer(&TL_TABLE, g_free); > + g_clear_pointer(&SIN_TABLE, g_free); > + g_clear_pointer(&AMS_TABLE, g_free); > + g_clear_pointer(&VIB_TABLE, g_free); > } > > /* CSM Key Control */ > @@ -1081,11 +1065,8 @@ FM_OPL *OPLCreate(int clock, int rate) > /* allocate OPL state space */ > state_size = sizeof(FM_OPL); > state_size += sizeof(OPL_CH)*max_ch; > - /* allocate memory block */ > - ptr = malloc(state_size); > - if(ptr==NULL) return NULL; > - /* clear */ > - memset(ptr,0,state_size); > + /* allocate memory block and zero-initialize */ > + ptr = g_malloc0(state_size); > OPL = (FM_OPL *)ptr; ptr+=sizeof(FM_OPL); > OPL->P_CH = (OPL_CH *)ptr; ptr+=sizeof(OPL_CH)*max_ch; > /* set channel state pointer */ > @@ -1128,7 +1109,7 @@ void OPLDestroy(FM_OPL *OPL) > } > #endif > OPL_UnLockTable(); > - free(OPL); > + g_free(OPL); > } thanks -- PMM
