From 11f2c86650fda8f1df606686799752223339b9e9 Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Mon, 4 Nov 2019 16:38:24 +1100 Subject: [PATCH] LU-9679 all: prefer sizeof(*var) for ALLOC/FREE The construct LIBCFS_ALLOC(var, sizeof(*var)); is more obviously correct than LIBCFS_ALLOC(var, sizeof(struct something)); and is preferred upstream (where it is actually kzalloc or similar of course). When it is that simple, and there is no multiplier for the size, CFS_ALLOC_PTR(var); is even better. The same logic applies to OBD_ALLOC(), LIBCFS_FREE(), and OBD_FREE(). So convert allocations and frees that use sizeof(struct..) to use one of the simpler constructs. In mgs_write_log_mdt0, uuid is better declared as a "struct obd_uuid *" which is a struct that contain a 'char' array. Test-Parameters: trivial Signed-off-by: Mr NeilBrown Change-Id: I8cd97c75241bbb87d15cc6b7c9ac2a7d6184d700 Reviewed-on: https://review.whamcloud.com/36661 Reviewed-by: Alex Zhuravlev Reviewed-by: James Simmons Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lnet/klnds/o2iblnd/o2iblnd.c | 4 ++-- lnet/lnet/lib-eq.c | 6 +++--- lnet/lnet/nidstrings.c | 8 ++++---- lnet/selftest/console.c | 8 ++++---- lustre/lfsck/lfsck_striped_dir.c | 2 +- lustre/llite/pcc.c | 8 ++++---- lustre/lod/lod_object.c | 2 +- lustre/mgs/mgs_llog.c | 13 +++++++------ lustre/obdecho/echo_client.c | 4 ++-- lustre/ofd/ofd_objects.c | 4 ++-- lustre/osd-zfs/osd_oi.c | 2 +- lustre/ptlrpc/client.c | 2 +- lustre/ptlrpc/nrs_tbf.c | 10 +++++----- lustre/target/out_handler.c | 5 ++--- 14 files changed, 39 insertions(+), 39 deletions(-) diff --git a/lnet/klnds/o2iblnd/o2iblnd.c b/lnet/klnds/o2iblnd/o2iblnd.c index 945f0c8..dd8b999 100644 --- a/lnet/klnds/o2iblnd/o2iblnd.c +++ b/lnet/klnds/o2iblnd/o2iblnd.c @@ -2294,7 +2294,7 @@ kiblnd_destroy_tx_pool(struct kib_pool *pool) pool->po_size * sizeof(struct kib_tx)); out: kiblnd_fini_pool(pool); - LIBCFS_FREE(tpo, sizeof(struct kib_tx_pool)); + CFS_FREE_PTR(tpo); } static int kiblnd_tx_pool_size(struct lnet_ni *ni, int ncpts) @@ -2330,7 +2330,7 @@ kiblnd_create_tx_pool(struct kib_poolset *ps, int size, struct kib_pool **pp_po) npg = (size * IBLND_MSG_SIZE + PAGE_SIZE - 1) / PAGE_SIZE; if (kiblnd_alloc_pages(&tpo->tpo_tx_pages, ps->ps_cpt, npg) != 0) { CERROR("Can't allocate tx pages: %d\n", npg); - LIBCFS_FREE(tpo, sizeof(struct kib_tx_pool)); + CFS_FREE_PTR(tpo); return -ENOMEM; } diff --git a/lnet/lnet/lib-eq.c b/lnet/lnet/lib-eq.c index 354c976..f5f2569 100644 --- a/lnet/lnet/lib-eq.c +++ b/lnet/lnet/lib-eq.c @@ -94,7 +94,7 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, return -ENOMEM; if (count != 0) { - LIBCFS_ALLOC(eq->eq_events, count * sizeof(struct lnet_event)); + LIBCFS_ALLOC(eq->eq_events, count * sizeof(*eq->eq_events)); if (eq->eq_events == NULL) goto failed; /* NB allocator has set all event sequence numbers to 0, @@ -128,7 +128,7 @@ LNetEQAlloc(unsigned int count, lnet_eq_handler_t callback, failed: if (eq->eq_events != NULL) - LIBCFS_FREE(eq->eq_events, count * sizeof(struct lnet_event)); + LIBCFS_FREE(eq->eq_events, count * sizeof(*eq->eq_events)); if (eq->eq_refs != NULL) cfs_percpt_free(eq->eq_refs); @@ -194,7 +194,7 @@ LNetEQFree(struct lnet_handle_eq eqh) lnet_res_unlock(LNET_LOCK_EX); if (events != NULL) - LIBCFS_FREE(events, size * sizeof(struct lnet_event)); + LIBCFS_FREE(events, size * sizeof(*events)); if (refs != NULL) cfs_percpt_free(refs); diff --git a/lnet/lnet/nidstrings.c b/lnet/lnet/nidstrings.c index 824bdf4..2061142 100644 --- a/lnet/lnet/nidstrings.c +++ b/lnet/lnet/nidstrings.c @@ -165,7 +165,7 @@ parse_addrange(const struct cfs_lstr *src, struct nidrange *nidrange) return 0; } - LIBCFS_ALLOC(addrrange, sizeof(struct addrrange)); + CFS_ALLOC_PTR(addrrange); if (addrrange == NULL) return -ENOMEM; list_add_tail(&addrrange->ar_link, &nidrange->nr_addrranges); @@ -222,7 +222,7 @@ add_nidrange(const struct cfs_lstr *src, return nr; } - LIBCFS_ALLOC(nr, sizeof(struct nidrange)); + CFS_ALLOC_PTR(nr); if (nr == NULL) return NULL; list_add_tail(&nr->nr_link, nidlist); @@ -283,7 +283,7 @@ free_addrranges(struct list_head *list) cfs_expr_list_free_list(&ar->ar_numaddr_ranges); list_del(&ar->ar_link); - LIBCFS_FREE(ar, sizeof(struct addrrange)); + CFS_FREE_PTR(ar); } } @@ -305,7 +305,7 @@ cfs_free_nidlist(struct list_head *list) nr = list_entry(pos, struct nidrange, nr_link); free_addrranges(&nr->nr_addrranges); list_del(pos); - LIBCFS_FREE(nr, sizeof(struct nidrange)); + CFS_FREE_PTR(nr); } } EXPORT_SYMBOL(cfs_free_nidlist); diff --git a/lnet/selftest/console.c b/lnet/selftest/console.c index 2428789..1721d65 100644 --- a/lnet/selftest/console.c +++ b/lnet/selftest/console.c @@ -802,7 +802,7 @@ lstcon_group_info(char *name, struct lstcon_ndlist_ent __user *gents_p, } /* non-verbose query */ - LIBCFS_ALLOC(gentp, sizeof(struct lstcon_ndlist_ent)); + CFS_ALLOC_PTR(gentp); if (gentp == NULL) { CERROR("Can't allocate ndlist_ent\n"); lstcon_group_decref(grp); @@ -816,7 +816,7 @@ lstcon_group_info(char *name, struct lstcon_ndlist_ent __user *gents_p, rc = copy_to_user(gents_p, gentp, sizeof(struct lstcon_ndlist_ent)) ? -EFAULT : 0; - LIBCFS_FREE(gentp, sizeof(struct lstcon_ndlist_ent)); + CFS_FREE_PTR(gentp); lstcon_group_decref(grp); @@ -966,7 +966,7 @@ lstcon_batch_info(char *name, struct lstcon_test_batch_ent __user *ent_up, } /* non-verbose query */ - LIBCFS_ALLOC(entp, sizeof(struct lstcon_test_batch_ent)); + CFS_ALLOC_PTR(entp); if (entp == NULL) return -ENOMEM; @@ -990,7 +990,7 @@ lstcon_batch_info(char *name, struct lstcon_test_batch_ent __user *ent_up, rc = copy_to_user(ent_up, entp, sizeof(struct lstcon_test_batch_ent)) ? -EFAULT : 0; - LIBCFS_FREE(entp, sizeof(struct lstcon_test_batch_ent)); + CFS_FREE_PTR(entp) return rc; } diff --git a/lustre/lfsck/lfsck_striped_dir.c b/lustre/lfsck/lfsck_striped_dir.c index b53e2f7..a222e6e 100644 --- a/lustre/lfsck/lfsck_striped_dir.c +++ b/lustre/lfsck/lfsck_striped_dir.c @@ -1358,7 +1358,7 @@ int lfsck_namespace_notify_lmv_master_local(const struct lu_env *env, else count = lmv4->lmv_stripe_count; - OBD_ALLOC_LARGE(lslr, sizeof(struct lfsck_slave_lmv_rec) * count); + OBD_ALLOC_LARGE(lslr, sizeof(*lslr) * count); if (lslr == NULL) { OBD_FREE_PTR(llu); diff --git a/lustre/llite/pcc.c b/lustre/llite/pcc.c index 9e5be20..d824c1b 100644 --- a/lustre/llite/pcc.c +++ b/lustre/llite/pcc.c @@ -214,13 +214,13 @@ pcc_fname_list_add(struct cfs_lstr *id, struct list_head *fname_list) { struct pcc_match_fname *fname; - OBD_ALLOC(fname, sizeof(struct pcc_match_fname)); + OBD_ALLOC_PTR(fname); if (fname == NULL) return -ENOMEM; OBD_ALLOC(fname->pmf_name, id->ls_len + 1); if (fname->pmf_name == NULL) { - OBD_FREE(fname, sizeof(struct pcc_match_fname)); + OBD_FREE_PTR(fname); return -ENOMEM; } @@ -313,7 +313,7 @@ pcc_expression_parse(struct cfs_lstr *src, struct list_head *cond_list) struct cfs_lstr field; int rc = 0; - OBD_ALLOC(expr, sizeof(struct pcc_expression)); + OBD_ALLOC_PTR(expr); if (expr == NULL) return -ENOMEM; @@ -371,7 +371,7 @@ pcc_conjunction_parse(struct cfs_lstr *src, struct list_head *cond_list) struct cfs_lstr expr; int rc = 0; - OBD_ALLOC(conjunction, sizeof(struct pcc_conjunction)); + OBD_ALLOC_PTR(conjunction); if (conjunction == NULL) return -ENOMEM; diff --git a/lustre/lod/lod_object.c b/lustre/lod/lod_object.c index 25ae4b2..2fcb93b 100644 --- a/lustre/lod/lod_object.c +++ b/lustre/lod/lod_object.c @@ -4483,7 +4483,7 @@ static int lod_layout_del_prep_layout(const struct lu_env *env, lu_object_put(env, &obj->do_lu); lod_comp->llc_stripe[j] = NULL; } - OBD_FREE(lod_comp->llc_stripe, sizeof(struct dt_object *) * + OBD_FREE(lod_comp->llc_stripe, sizeof(*lod_comp->llc_stripe) * lod_comp->llc_stripes_allocated); lod_comp->llc_stripe = NULL; OBD_FREE(lod_comp->llc_ost_indices, diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c index b9d8550..0ac9a7a 100644 --- a/lustre/mgs/mgs_llog.c +++ b/lustre/mgs/mgs_llog.c @@ -2725,13 +2725,14 @@ static int mgs_write_log_mdt0(const struct lu_env *env, { char *log = mti->mti_svname; struct llog_handle *llh = NULL; - char *uuid, *lovname; + struct obd_uuid *uuid; + char *lovname; char mdt_index[6]; char *ptr = mti->mti_params; int rc = 0, failout = 0; ENTRY; - OBD_ALLOC(uuid, sizeof(struct obd_uuid)); + OBD_ALLOC_PTR(uuid); if (uuid == NULL) RETURN(-ENOMEM); @@ -2756,17 +2757,17 @@ static int mgs_write_log_mdt0(const struct lu_env *env, /* add MDT itself */ /* FIXME this whole fn should be a single journal transaction */ - sprintf(uuid, "%s_UUID", log); + sprintf(uuid->uuid, "%s_UUID", log); rc = record_marker(env, llh, fsdb, CM_START, log, "add mdt"); if (rc) GOTO(out_lod, rc); - rc = record_attach(env, llh, log, LUSTRE_MDT_NAME, uuid); + rc = record_attach(env, llh, log, LUSTRE_MDT_NAME, uuid->uuid); if (rc) GOTO(out_end, rc); rc = record_mount_opt(env, llh, log, lovname, NULL); if (rc) GOTO(out_end, rc); - rc = record_setup(env, llh, log, uuid, mdt_index, lovname, + rc = record_setup(env, llh, log, uuid->uuid, mdt_index, lovname, failout ? "n" : "f"); if (rc) GOTO(out_end, rc); @@ -2778,7 +2779,7 @@ out_end: out_lod: name_destroy(&lovname); out_free: - OBD_FREE(uuid, sizeof(struct obd_uuid)); + OBD_FREE_PTR(uuid); RETURN(rc); } diff --git a/lustre/obdecho/echo_client.c b/lustre/obdecho/echo_client.c index 4b430c5..790e2fb 100644 --- a/lustre/obdecho/echo_client.c +++ b/lustre/obdecho/echo_client.c @@ -2650,7 +2650,7 @@ static int echo_client_prep_commit(const struct lu_env *env, apc = npages = batch >> PAGE_SHIFT; tot_pages = count >> PAGE_SHIFT; - OBD_ALLOC_LARGE(lnb, apc * sizeof(struct niobuf_local)); + OBD_ALLOC_LARGE(lnb, apc * sizeof(*lnb)); if (!lnb) RETURN(-ENOMEM); @@ -2716,7 +2716,7 @@ static int echo_client_prep_commit(const struct lu_env *env, } out: - OBD_FREE_LARGE(lnb, apc * sizeof(struct niobuf_local)); + OBD_FREE_LARGE(lnb, apc * sizeof(*lnb)); RETURN(ret); } diff --git a/lustre/ofd/ofd_objects.c b/lustre/ofd/ofd_objects.c index 4c76972..8579158 100644 --- a/lustre/ofd/ofd_objects.c +++ b/lustre/ofd/ofd_objects.c @@ -280,7 +280,7 @@ int ofd_precreate_objects(const struct lu_env *env, struct ofd_device *ofd, RETURN(rc = -ENOSPC); } - OBD_ALLOC(batch, nr_saved * sizeof(struct ofd_object *)); + OBD_ALLOC(batch, nr_saved * sizeof(*batch)); if (batch == NULL) RETURN(-ENOMEM); @@ -462,7 +462,7 @@ out: continue; ofd_object_put(env, fo); } - OBD_FREE(batch, nr_saved * sizeof(struct ofd_object *)); + OBD_FREE(batch, nr_saved * sizeof(*batch)); CDEBUG((objects == 0 && rc == 0) ? D_ERROR : D_OTHER, "created %d/%d objects: %d\n", objects, nr_saved, rc); diff --git a/lustre/osd-zfs/osd_oi.c b/lustre/osd-zfs/osd_oi.c index efcafe4..e4244dc 100644 --- a/lustre/osd-zfs/osd_oi.c +++ b/lustre/osd-zfs/osd_oi.c @@ -922,7 +922,7 @@ int osd_oi_init(const struct lu_env *env, struct osd_device *o) open: LASSERT((count & (count - 1)) == 0); o->od_oi_count = count; - OBD_ALLOC(o->od_oi_table, sizeof(struct osd_oi *) * count); + OBD_ALLOC(o->od_oi_table, sizeof(*o->od_oi_table) * count); if (o->od_oi_table == NULL) GOTO(out, rc = -ENOMEM); diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c index eccf0a8..cf2631a 100644 --- a/lustre/ptlrpc/client.c +++ b/lustre/ptlrpc/client.c @@ -624,7 +624,7 @@ ptlrpc_init_rq_pool(int num_rq, int msgsize, { struct ptlrpc_request_pool *pool; - OBD_ALLOC(pool, sizeof(struct ptlrpc_request_pool)); + OBD_ALLOC_PTR(pool); if (!pool) return NULL; diff --git a/lustre/ptlrpc/nrs_tbf.c b/lustre/ptlrpc/nrs_tbf.c index d5fab11..9486172 100644 --- a/lustre/ptlrpc/nrs_tbf.c +++ b/lustre/ptlrpc/nrs_tbf.c @@ -807,7 +807,7 @@ nrs_tbf_jobid_list_free(struct list_head *jobid_list) list_for_each_entry_safe(jobid, n, jobid_list, tj_linkage) { OBD_FREE(jobid->tj_id, strlen(jobid->tj_id) + 1); list_del(&jobid->tj_linkage); - OBD_FREE(jobid, sizeof(struct nrs_tbf_jobid)); + OBD_FREE_PTR(jobid); } } @@ -817,13 +817,13 @@ nrs_tbf_jobid_list_add(struct cfs_lstr *id, struct list_head *jobid_list) struct nrs_tbf_jobid *jobid; char *ptr; - OBD_ALLOC(jobid, sizeof(struct nrs_tbf_jobid)); + OBD_ALLOC_PTR(jobid); if (jobid == NULL) return -ENOMEM; OBD_ALLOC(jobid->tj_id, id->ls_len + 1); if (jobid->tj_id == NULL) { - OBD_FREE(jobid, sizeof(struct nrs_tbf_jobid)); + OBD_FREE_PTR(jobid); return -ENOMEM; } @@ -1840,7 +1840,7 @@ nrs_tbf_expression_parse(struct cfs_lstr *src, struct list_head *cond_list) struct cfs_lstr field; int rc = 0; - OBD_ALLOC(expr, sizeof(struct nrs_tbf_expression)); + OBD_ALLOC_PTR(expr); if (expr == NULL) return -ENOMEM; @@ -1903,7 +1903,7 @@ nrs_tbf_conjunction_parse(struct cfs_lstr *src, struct list_head *cond_list) struct cfs_lstr expr; int rc = 0; - OBD_ALLOC(conjunction, sizeof(struct nrs_tbf_conjunction)); + OBD_ALLOC_PTR(conjunction); if (conjunction == NULL) return -ENOMEM; diff --git a/lustre/target/out_handler.c b/lustre/target/out_handler.c index c3efb6f..b18937e 100644 --- a/lustre/target/out_handler.c +++ b/lustre/target/out_handler.c @@ -715,8 +715,7 @@ static int out_read(struct tgt_session_info *tsi) orr = (struct out_read_reply *)update_result->our_data; nbufs = (size + OUT_BULK_BUFFER_SIZE - 1) / OUT_BULK_BUFFER_SIZE; - OBD_ALLOC(rdbuf, sizeof(struct lu_rdbuf) + - nbufs * sizeof(rdbuf->rb_bufs[0])); + OBD_ALLOC(rdbuf, sizeof(*rdbuf) + nbufs * sizeof(rdbuf->rb_bufs[0])); if (rdbuf == NULL) GOTO(out, rc = -ENOMEM); @@ -761,7 +760,7 @@ out_free: rdbuf->rb_bufs[i].lb_len); } } - OBD_FREE(rdbuf, sizeof(struct lu_rdbuf) + + OBD_FREE(rdbuf, sizeof(*rdbuf) + nbufs * sizeof(rdbuf->rb_bufs[0])); out: /* Insert read buffer */ -- 1.8.3.1