Re: [PATCH 02/12] hw/mem/cxl_type3: Drop handling of failure of g_malloc0()

[fan]

On Wed, Jan 24, 2024 at 12:40:50PM +0000, Jonathan Cameron wrote:
> As g_malloc0 will just exit QEMU on failure there is no point
> in checking for it failing.

The change is also related to g_malloc. So we may want to also mention it in
the comments like " As g_malloc and g_malloc0 will just ....  ". Other
than that, LGTM.

Reviewed-by: Fan Ni <fan.ni@samsung.com>

Fan

> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  hw/mem/cxl_type3.c | 52 +++++++---------------------------------------
>  1 file changed, 7 insertions(+), 45 deletions(-)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 52647b4ac7..1b92a065a3 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -42,9 +42,9 @@ enum {
>      CT3_CDAT_NUM_ENTRIES
>  };
>  
> -static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table,
> -                                         int dsmad_handle, MemoryRegion *mr,
> -                                         bool is_pmem, uint64_t dpa_base)
> +static void ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table,
> +                                          int dsmad_handle, MemoryRegion *mr,
> +                                          bool is_pmem, uint64_t dpa_base)
>  {
>      g_autofree CDATDsmas *dsmas = NULL;
>      g_autofree CDATDslbis *dslbis0 = NULL;
> @@ -54,9 +54,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      g_autofree CDATDsemts *dsemts = NULL;
>  
>      dsmas = g_malloc(sizeof(*dsmas));
> -    if (!dsmas) {
> -        return -ENOMEM;
> -    }
>      *dsmas = (CDATDsmas) {
>          .header = {
>              .type = CDAT_TYPE_DSMAS,
> @@ -70,9 +67,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>  
>      /* For now, no memory side cache, plausiblish numbers */
>      dslbis0 = g_malloc(sizeof(*dslbis0));
> -    if (!dslbis0) {
> -        return -ENOMEM;
> -    }
>      *dslbis0 = (CDATDslbis) {
>          .header = {
>              .type = CDAT_TYPE_DSLBIS,
> @@ -86,9 +80,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      };
>  
>      dslbis1 = g_malloc(sizeof(*dslbis1));
> -    if (!dslbis1) {
> -        return -ENOMEM;
> -    }
>      *dslbis1 = (CDATDslbis) {
>          .header = {
>              .type = CDAT_TYPE_DSLBIS,
> @@ -102,9 +93,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      };
>  
>      dslbis2 = g_malloc(sizeof(*dslbis2));
> -    if (!dslbis2) {
> -        return -ENOMEM;
> -    }
>      *dslbis2 = (CDATDslbis) {
>          .header = {
>              .type = CDAT_TYPE_DSLBIS,
> @@ -118,9 +106,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      };
>  
>      dslbis3 = g_malloc(sizeof(*dslbis3));
> -    if (!dslbis3) {
> -        return -ENOMEM;
> -    }
>      *dslbis3 = (CDATDslbis) {
>          .header = {
>              .type = CDAT_TYPE_DSLBIS,
> @@ -134,9 +119,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      };
>  
>      dsemts = g_malloc(sizeof(*dsemts));
> -    if (!dsemts) {
> -        return -ENOMEM;
> -    }
>      *dsemts = (CDATDsemts) {
>          .header = {
>              .type = CDAT_TYPE_DSEMTS,
> @@ -159,8 +141,6 @@ static int ct3_build_cdat_entries_for_mr(CDATSubHeader 
> **cdat_table,
>      cdat_table[CT3_CDAT_DSLBIS2] = g_steal_pointer(&dslbis2);
>      cdat_table[CT3_CDAT_DSLBIS3] = g_steal_pointer(&dslbis3);
>      cdat_table[CT3_CDAT_DSEMTS] = g_steal_pointer(&dsemts);
> -
> -    return 0;
>  }
>  
>  static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
> @@ -171,7 +151,6 @@ static int ct3_build_cdat_table(CDATSubHeader 
> ***cdat_table, void *priv)
>      int dsmad_handle = 0;
>      int cur_ent = 0;
>      int len = 0;
> -    int rc, i;
>  
>      if (!ct3d->hostpmem && !ct3d->hostvmem) {
>          return 0;
> @@ -194,27 +173,18 @@ static int ct3_build_cdat_table(CDATSubHeader 
> ***cdat_table, void *priv)
>      }
>  
>      table = g_malloc0(len * sizeof(*table));
> -    if (!table) {
> -        return -ENOMEM;
> -    }
>  
>      /* Now fill them in */
>      if (volatile_mr) {
> -        rc = ct3_build_cdat_entries_for_mr(table, dsmad_handle++, 
> volatile_mr,
> -                                           false, 0);
> -        if (rc < 0) {
> -            return rc;
> -        }
> +        ct3_build_cdat_entries_for_mr(table, dsmad_handle++, volatile_mr,
> +                                      false, 0);
>          cur_ent = CT3_CDAT_NUM_ENTRIES;
>      }
>  
>      if (nonvolatile_mr) {
>          uint64_t base = volatile_mr ? memory_region_size(volatile_mr) : 0;
> -        rc = ct3_build_cdat_entries_for_mr(&(table[cur_ent]), dsmad_handle++,
> -                                           nonvolatile_mr, true, base);
> -        if (rc < 0) {
> -            goto error_cleanup;
> -        }
> +        ct3_build_cdat_entries_for_mr(&(table[cur_ent]), dsmad_handle++,
> +                                      nonvolatile_mr, true, base);
>          cur_ent += CT3_CDAT_NUM_ENTRIES;
>      }
>      assert(len == cur_ent);
> @@ -222,11 +192,6 @@ static int ct3_build_cdat_table(CDATSubHeader 
> ***cdat_table, void *priv)
>      *cdat_table = g_steal_pointer(&table);
>  
>      return len;
> -error_cleanup:
> -    for (i = 0; i < cur_ent; i++) {
> -        g_free(table[i]);
> -    }
> -    return rc;
>  }
>  
>  static void ct3_free_cdat_table(CDATSubHeader **cdat_table, int num, void 
> *priv)
> @@ -1168,9 +1133,6 @@ void qmp_cxl_inject_uncorrectable_errors(const char 
> *path,
>          }
>  
>          cxl_err = g_malloc0(sizeof(*cxl_err));
> -        if (!cxl_err) {
> -            return;
> -        }
>  
>          cxl_err->type = cxl_err_code;
>          while (header && header_count < 32) {
> -- 
> 2.39.2
>