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...@samsung.com>
Fan
>
> Signed-off-by: Jonathan Cameron <jonathan.came...@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
>
