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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;