From: adilger Date: Fri, 29 Mar 2002 22:29:59 +0000 (+0000) Subject: A bunch of minor cleanups when using sizeof(). This even appears to have X-Git-Tag: v1_7_100~5829 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=b086032ef025866e71e001281cc64d900b95bcc2;p=fs%2Flustre-release.git A bunch of minor cleanups when using sizeof(). This even appears to have caught 3 minor bugs in the code. Prompted by a couple of similar bugs that were just found in the kernel. Basically, with a variable like "struct foo *bar" it is preferrable to use "sizeof(*bar)" everywhere instead of "sizeof(struct foo)". The former isolates us from errors in case the type of "bad" ever changes. Apparent bugs were in filter/filter_obd.c and ldlm/ldlm_resource.c (we were using __u32 when the variable was declared as __u64). There also appears to be a memory leak in ldlm_resource.c. --- diff --git a/lustre/ldlm/ldlm_lock.c b/lustre/ldlm/ldlm_lock.c index 34a861d..137df0d 100644 --- a/lustre/ldlm/ldlm_lock.c +++ b/lustre/ldlm/ldlm_lock.c @@ -190,7 +190,7 @@ ldlm_error_t ldlm_local_lock_enqueue(struct obd_device *obddev, rc = policy(parent_res, res_id, new_id, mode, NULL); if (rc == ELDLM_RES_CHANGED) { *flags |= LDLM_FL_RES_CHANGED; - memcpy(res_id, new_id, sizeof(__u64) * RES_NAME_SIZE); + memcpy(res_id, new_id, sizeof(*new_id)); } } diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c index db33639..4d8ed88 100644 --- a/lustre/ldlm/ldlm_resource.c +++ b/lustre/ldlm/ldlm_resource.c @@ -54,7 +54,8 @@ static void res_hash_init(struct ldlm_namespace *ns) if (ns->ns_hash != NULL) return; - OBD_ALLOC(res_hash, sizeof(struct list_head) * RES_HASH_SIZE); + /* FIXME: this memory appears to be leaked */ + OBD_ALLOC(res_hash, sizeof(*res_hash) * RES_HASH_SIZE); if (!res_hash) LBUG(); @@ -148,7 +149,7 @@ static struct ldlm_resource *ldlm_resource_add(struct ldlm_namespace *ns, if (!res) LBUG(); - memcpy(res->lr_name, name, RES_NAME_SIZE * sizeof(__u32)); + memcpy(res->lr_name, name, sizeof(res->lr_name)); res->lr_namespace = ns; if (type < 0 || type > LDLM_MAX_TYPE) LBUG(); @@ -187,8 +188,7 @@ struct ldlm_resource *ldlm_resource_get(struct ldlm_namespace *ns, struct ldlm_resource *chk; chk = list_entry(tmp, struct ldlm_resource, lr_hash); - if (memcmp(chk->lr_name, name, - RES_NAME_SIZE * sizeof(__u32)) == 0) { + if (memcmp(chk->lr_name, name, sizeof(chk->lr_name)) == 0) { res = chk; atomic_inc(&res->lr_refcount); break; diff --git a/lustre/llite/super.c b/lustre/llite/super.c index 5977ebe..2a829d6 100644 --- a/lustre/llite/super.c +++ b/lustre/llite/super.c @@ -203,7 +203,7 @@ static void ll_put_super(struct super_block *sb) struct ll_sb_info *sbi = sb->u.generic_sbp; ENTRY; obd_disconnect(&sbi->ll_conn); - OBD_FREE(sb->u.generic_sbp, sizeof(struct ll_sb_info)); + OBD_FREE(sb->u.generic_sbp, sizeof(*sbi)); MOD_DEC_USE_COUNT; EXIT; } /* ll_put_super */ diff --git a/lustre/obdclass/class_obd.c b/lustre/obdclass/class_obd.c index 81f0640..a9fa49d 100644 --- a/lustre/obdclass/class_obd.c +++ b/lustre/obdclass/class_obd.c @@ -387,10 +387,10 @@ static int obd_class_ioctl (struct inode * inode, struct file * filp, CDEBUG(D_INODE, "BRW %s with %dx%d pages\n", rw == OBD_BRW_READ ? "read" : "write", num, oa_bufs[0]); - bufs = kmalloc(pages * sizeof(struct page *), GFP_KERNEL); - counts = kmalloc(pages * sizeof(obd_size), GFP_KERNEL); - offsets = kmalloc(pages * sizeof(obd_off), GFP_KERNEL); - flags = kmalloc(pages * sizeof(obd_flag), GFP_KERNEL); + bufs = kmalloc(pages * sizeof(*bufs), GFP_KERNEL); + counts = kmalloc(pages * sizeof(*counts), GFP_KERNEL); + offsets = kmalloc(pages * sizeof(*offsets), GFP_KERNEL); + flags = kmalloc(pages * sizeof(*flags), GFP_KERNEL); if (!bufs || !counts || !offsets || !flags) { CERROR("no memory for %d BRW per-page data\n", pages); err = -ENOMEM; @@ -553,12 +553,12 @@ static int __init init_obdclass(void) int i; printk(KERN_INFO "OBD class driver v0.01, braam@stelias.com\n"); - + INIT_LIST_HEAD(&obd_types); - - if ( (err = misc_register(&obd_psdev)) ) { + + if ((err = misc_register(&obd_psdev))) { CERROR("cannot register %d err %d\n", OBD_MINOR, err); - return -EIO; + return err; } for (i = 0; i < MAX_OBD_DEVICES; i++) { diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index 64b6db4..4a366c7 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -89,7 +89,7 @@ int gen_connect (struct obd_conn *conn) { struct obd_client * cli; - OBD_ALLOC(cli, sizeof(struct obd_client)); + OBD_ALLOC(cli, sizeof(*cli)); if ( !cli ) { CERROR("no memory! (minor %d)\n", conn->oc_dev->obd_minor); return -ENOMEM; @@ -121,7 +121,7 @@ int gen_disconnect(struct obd_conn *conn) list_del(&(cli->cli_chain)); - OBD_FREE(cli, sizeof(struct obd_client)); + OBD_FREE(cli, sizeof(*cli)); CDEBUG(D_INFO, "disconnect: ID %u\n", conn->oc_id); diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index 4e5b673..2e4c6fc 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -411,12 +411,12 @@ int osc_brw_read(struct obd_conn *conn, obd_count num_oa, struct obdo **oa, } request->rq_req.ost->cmd = OBD_BRW_READ; - OBD_ALLOC(bulk, pages * sizeof(struct ptlrpc_bulk_desc *)); + OBD_ALLOC(bulk, pages * sizeof(*bulk)); if (bulk == NULL) { CERROR("cannot alloc bulk desc vector\n"); return -ENOMEM; } - memset(bulk, 0, pages * sizeof(struct ptlrpc_bulk_desc *)); + memset(bulk, 0, pages * sizeof(*bulk)); n = 0; ptr1 = ost_req_buf1(request->rq_req.ost); @@ -460,12 +460,12 @@ int osc_brw_read(struct obd_conn *conn, obd_count num_oa, struct obdo **oa, if (bulk[n] == NULL) continue; kunmap(buf[n]); - OBD_FREE(bulk[n], sizeof(struct ptlrpc_bulk_desc)); + OBD_FREE(bulk[n], sizeof(**bulk)); n++; } } - OBD_FREE(bulk, pages * sizeof(struct ptlrpc_bulk_desc *)); + OBD_FREE(bulk, pages * sizeof(*bulk)); ptlrpc_free_req(request); return rc; } diff --git a/lustre/ost/ost_handler.c b/lustre/ost/ost_handler.c index f74d346..67a1653 100644 --- a/lustre/ost/ost_handler.c +++ b/lustre/ost/ost_handler.c @@ -298,9 +298,8 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) int i, j; int objcount, niocount; char *tmp1, *tmp2, *end2; - char *res = NULL; int cmd; - struct niobuf *nb, *src, *dst; + struct niobuf *nb, *dst, *res = NULL; struct obd_ioobj *ioo; struct ost_req *r = req->rq_req.ost; @@ -334,7 +333,7 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) CERROR("cannot pack reply\n"); RETURN(rc); } - OBD_ALLOC(res, sizeof(struct niobuf) * niocount); + OBD_ALLOC(res, sizeof(*res) * niocount); if (res == NULL) RETURN(-ENOMEM); @@ -343,7 +342,7 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) tmp2 = ost_req_buf2(r); req->rq_rep.ost->result = obd_preprw (cmd, &conn, objcount, (struct obd_ioobj *)tmp1, - niocount, (struct niobuf *)tmp2, (struct niobuf *)res); + niocount, (struct niobuf *)tmp2, res); if (req->rq_rep.ost->result) GOTO(out, 0); @@ -356,10 +355,9 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) GOTO(out, rc); } - src = &((struct niobuf *)res)[i]; dst = &((struct niobuf *)tmp2)[i]; bulk->b_xid = dst->xid; - bulk->b_buf = (void *)(unsigned long)src->addr; + bulk->b_buf = (void *)(unsigned long)res[i].addr; bulk->b_buflen = PAGE_SIZE; rc = ptlrpc_send_bulk(bulk, OST_BULK_PORTAL); if (rc) @@ -387,12 +385,12 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) tmp2 = ost_req_buf2(r); req->rq_rep.ost->result = obd_commitrw (cmd, &conn, objcount, (struct obd_ioobj *)tmp1, - niocount, (struct niobuf *)res); + niocount, res); EXIT; out: if (res != NULL) - OBD_FREE(res, sizeof(struct niobuf) * niocount); + OBD_FREE(res, sizeof(*res) * niocount); if (bulk != NULL) OBD_FREE(bulk, sizeof(*bulk)); if (bulk_vec != NULL) { @@ -400,8 +398,7 @@ static int ost_brw_read(struct ost_obd *obddev, struct ptlrpc_request *req) if (bulk_vec[i] != NULL) OBD_FREE(bulk_vec[i], sizeof(*bulk)); } - OBD_FREE(bulk_vec, - niocount * sizeof(struct ptlrpc_bulk_desc *)); + OBD_FREE(bulk_vec, niocount * sizeof(*bulk_vec)); } return 0;