1. Patchew
  2. QEMU
  3. [PATCH 00/12 qemu] CXL emulation fixes and minor cleanup.
  4. View patch

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

Jonathan Cameron via posted 12 patches 10 months, 1 week ago
Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
There is a newer version of this series
[PATCH 02/12] hw/mem/cxl_type3: Drop handling of failure of g_malloc0()
Posted by Jonathan Cameron via 10 months, 1 week ago 
As g_malloc0 will just exit QEMU on failure there is no point
in checking for it failing.

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
Re: [PATCH 02/12] hw/mem/cxl_type3: Drop handling of failure of g_malloc0()
Posted by fan 10 months ago 
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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.