[PATCH] hw/audio/fmopl: Convert malloc/free to g_malloc/g_free

AADHYA R posted 1 patch 3 weeks, 2 days ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/audio/fmopl.c | 41 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 29 deletions(-)
[PATCH] hw/audio/fmopl: Convert malloc/free to g_malloc/g_free
Posted by AADHYA R 3 weeks, 2 days ago
As per the QEMU coding style guide, replace standard C malloc()
and free() with GLib's g_malloc() and g_free().

Since g_malloc() safely aborts on out-of-memory errors, the
redundant NULL checks and their associated error-handling paths
have been removed.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1798

Signed-off-by: Aadhya-R <aadhyar.cs24@rvce.edu.in>
---
 hw/audio/fmopl.c | 41 ++++++++++++-----------------------------
 1 file changed, 12 insertions(+), 29 deletions(-)

diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
index a63ad0f04d..79e43d5410 100644
--- a/hw/audio/fmopl.c
+++ b/hw/audio/fmopl.c
@@ -50,7 +50,6 @@ static int opl_dbg_maxchip,opl_dbg_chip;
 /* attack/decay rate time rate */
 #define OPL_ARRATE     141280  /* RATE 4 =  2826.24ms @ 3.6MHz */
 #define OPL_DRRATE    1956000  /* RATE 4 = 39280.64ms @ 3.6MHz */
-
 #define DELTAT_MIXING_LEVEL (1) /* DELTA-T ADPCM MIXING LEVEL */

 #define FREQ_BITS 24                   /* frequency turn          */
@@ -607,26 +606,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_malloc(TL_MAX * 2 * sizeof(int32_t));
+    SIN_TABLE = g_malloc(SIN_ENT * 4 * sizeof(int32_t *));
+    AMS_TABLE = g_malloc(AMS_ENT * 2 * sizeof(int32_t));
+    VIB_TABLE = g_malloc(VIB_ENT * 2 * sizeof(int32_t));
      ENV_CURVE = g_new(int32_t, 2 * EG_ENT + 1);
         /* make total level table */
         for (t = 0;t < EG_ENT-1 ;t++){
@@ -696,10 +679,10 @@ 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_free(TL_TABLE);
+    g_free(SIN_TABLE);
+    g_free(AMS_TABLE);
+    g_free(VIB_TABLE);
 }

 /* CSM Key Control */
@@ -1082,9 +1065,9 @@ FM_OPL *OPLCreate(int clock, int rate)
         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 */
+    ptr = g_malloc(state_size);
+
+    /* clear */
         memset(ptr,0,state_size);
         OPL        = (FM_OPL *)ptr; ptr+=sizeof(FM_OPL);
         OPL->P_CH  = (OPL_CH *)ptr; ptr+=sizeof(OPL_CH)*max_ch;
@@ -1128,7 +1111,7 @@ void OPLDestroy(FM_OPL *OPL)
         }
 #endif
         OPL_UnLockTable();
-       free(OPL);
+    g_free(OPL);
 }

 /* ----------  Option handlers ----------       */
--
2.43.0
Re: [PATCH] hw/audio/fmopl: Convert malloc/free to g_malloc/g_free
Posted by Richard Henderson 2 weeks, 4 days ago
On 3/15/26 08:29, AADHYA R wrote:
> +    TL_TABLE = g_malloc(TL_MAX * 2 * sizeof(int32_t));
> +    SIN_TABLE = g_malloc(SIN_ENT * 4 * sizeof(int32_t *));
> +    AMS_TABLE = g_malloc(AMS_ENT * 2 * sizeof(int32_t));
> +    VIB_TABLE = g_malloc(VIB_ENT * 2 * sizeof(int32_t));

Better as

	g_new(int32_t, TL_MAX * 2);

etc.


r~
Re: [PATCH] hw/audio/fmopl: Convert malloc/free to g_malloc/g_free
Posted by Alex Bennée 3 weeks, 1 day ago
AADHYA R <aadhyar.cs24@rvce.edu.in> writes:

> As per the QEMU coding style guide, replace standard C malloc()
> and free() with GLib's g_malloc() and g_free().
>
> Since g_malloc() safely aborts on out-of-memory errors, the
> redundant NULL checks and their associated error-handling paths
> have been removed.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1798
>
> Signed-off-by: Aadhya-R <aadhyar.cs24@rvce.edu.in>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Although the whole file is fairly out of sync with modern QEMU
standards. Other things to consider:

  - #if 0 sections
  - replace hand-written logs in favour of tracepoints where appropriate
  - coding style clean-ups (mostly mandatory {}'s)

Unfortunately the whole file is tabbed. Usually we keep cosmetic commits
separate from logic changes. 

It would be nice to find something that exercised the YM emulation of
the adlib card. Are you running anything in particular?

> ---
>  hw/audio/fmopl.c | 41 ++++++++++++-----------------------------
>  1 file changed, 12 insertions(+), 29 deletions(-)
>
> diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
> index a63ad0f04d..79e43d5410 100644
> --- a/hw/audio/fmopl.c
> +++ b/hw/audio/fmopl.c
> @@ -50,7 +50,6 @@ static int opl_dbg_maxchip,opl_dbg_chip;
>  /* attack/decay rate time rate */
>  #define OPL_ARRATE     141280  /* RATE 4 =  2826.24ms @ 3.6MHz */
>  #define OPL_DRRATE    1956000  /* RATE 4 = 39280.64ms @ 3.6MHz */
> -
>  #define DELTAT_MIXING_LEVEL (1) /* DELTA-T ADPCM MIXING LEVEL */
>
>  #define FREQ_BITS 24                   /* frequency turn          */
> @@ -607,26 +606,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_malloc(TL_MAX * 2 * sizeof(int32_t));
> +    SIN_TABLE = g_malloc(SIN_ENT * 4 * sizeof(int32_t *));
> +    AMS_TABLE = g_malloc(AMS_ENT * 2 * sizeof(int32_t));
> +    VIB_TABLE = g_malloc(VIB_ENT * 2 * sizeof(int32_t));
>       ENV_CURVE = g_new(int32_t, 2 * EG_ENT + 1);
>          /* make total level table */
>          for (t = 0;t < EG_ENT-1 ;t++){
> @@ -696,10 +679,10 @@ 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_free(TL_TABLE);
> +    g_free(SIN_TABLE);
> +    g_free(AMS_TABLE);
> +    g_free(VIB_TABLE);
>  }
>
>  /* CSM Key Control */
> @@ -1082,9 +1065,9 @@ FM_OPL *OPLCreate(int clock, int rate)
>          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 */
> +    ptr = g_malloc(state_size);
> +
> +    /* clear */
>          memset(ptr,0,state_size);
>          OPL        = (FM_OPL *)ptr; ptr+=sizeof(FM_OPL);
>          OPL->P_CH  = (OPL_CH *)ptr; ptr+=sizeof(OPL_CH)*max_ch;
> @@ -1128,7 +1111,7 @@ void OPLDestroy(FM_OPL *OPL)
>          }
>  #endif
>          OPL_UnLockTable();
> -       free(OPL);
> +    g_free(OPL);
>  }
>
>  /* ----------  Option handlers ----------       */

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro