Re: [PATCH v2] hw/audio/fmopl.c: Convert malloc, free to g_malloc0, g_new and g_free

On 28.04.2026 19:20, Lucas Cardoso 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(-)
> 
> diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c

> +    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);


How about this:

diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
index a63ad0f04d..91a41f5fa9 100644
--- a/hw/audio/fmopl.c
+++ b/hw/audio/fmopl.c
@@ -175,18 +175,18 @@ static const int32_t SL_TABLE[16]={
  /* TotalLevel : 48 24 12  6  3 1.5 0.75 (dB) */
  /* TL_TABLE[ 0      to TL_MAX          ] : plus  section */
  /* TL_TABLE[ TL_MAX to TL_MAX+TL_MAX-1 ] : minus section */
-static int32_t *TL_TABLE;
+static int32_t TL_TABLE[TL_MAX*2];

  /* pointers to TL_TABLE with sinwave output offset */
-static int32_t **SIN_TABLE;
+static int32_t *SIN_TABLE[SIN_ENT * 4];

  /* LFO table */
-static int32_t *AMS_TABLE;
-static int32_t *VIB_TABLE;
+static int32_t AMS_TABLE[AMS_ENT * 2];
+static int32_t VIB_TABLE[VIB_ENT * 2];

  /* envelope output curve table */
  /* attack + decay + OFF */
-static int32_t *ENV_CURVE;
+static int32_t ENV_CURVE[2 * EG_ENT + 1];

  /* multiple table */
  #define ML 2
@@ -606,28 +606,6 @@ static int OPLOpenTable( void )
         int i,j;
         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;
-       }
-    ENV_CURVE = g_new(int32_t, 2 * EG_ENT + 1);
         /* make total level table */
         for (t = 0;t < EG_ENT-1 ;t++){
                 rate = ((1<<TL_BITS)-1)/pow(10,EG_STEP*t/20);   /* dB 
-> voltage */
@@ -693,15 +671,6 @@ static int OPLOpenTable( void )
  }


-static void OPLCloseTable( void )
-{
-    g_free(ENV_CURVE);
-       free(TL_TABLE);
-       free(SIN_TABLE);
-       free(AMS_TABLE);
-       free(VIB_TABLE);
-}
-
  /* CSM Key Control */
  static inline void CSMKeyControll(OPL_CH *CH)
  {
@@ -970,7 +939,6 @@ static void OPL_UnLockTable(void)
         if(num_lock) return;
         /* last time */
         cur_chip = NULL;
-       OPLCloseTable();
  }

  /*******************************************************************************/

?

> @@ -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);
>   }

And this is a 2-piece allocation in one go.  Which is,
by the way, is also fixed-size (state_size is constant).
I'd either split it into two separate allocations, or better yet,
made OPL->P_CH an array (instead of a pointer.

This whole file needs quite some care.  Incomplete or wrong or out of
place comments, mix of different coding styles, tricky allocations like
this one which don't need to be tricky, etc..  Many different things.

I dunno...

/mjt