Re: RFC: turn kmalloc+memset(,0,) into kcalloc

[Paul Jackson <pj@engr.sgi.com>]

Paulo wrote:
> In the first case you have to read carefully to make sure that the size 
> argument in both the kmalloc and the memset are the same.

If that were the only concern (which it isn't, and I don't pretend to be
addressing the other concerns on this thread) then pulling out the
common sub-expression would help readability, and reduce the chance of a
coding bug should the size change in the future.

Anytime you find yourself coding the same complex expression twice, an
alarm should go off in your mind, and you should ask yourself if there
is a reasonable way to get by with only one instance of the coding in
the source.

The following patch shows what I mean here.

It also replaces the less conventional form:

	if (!(ptr = some_function(args)) do-something;

with the more conventional form:

	if ((ptr = some_function(args)) == NULL)
		do-something;


diff -Naurp 2.6.12-rc2.orig/drivers/usb/input/hid-core.c 2.6.12-rc2/drivers/usb/input/hid-core.c
--- 2.6.12-rc2.orig/drivers/usb/input/hid-core.c	2005-04-09 06:57:47.000000000 -0700
+++ 2.6.12-rc2/drivers/usb/input/hid-core.c	2005-04-09 07:05:14.000000000 -0700
@@ -90,17 +90,18 @@ static struct hid_report *hid_register_r
 static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned values)
 {
 	struct hid_field *field;
+	int sz;
 
 	if (report->maxfield == HID_MAX_FIELDS) {
 		dbg("too many fields in report");
 		return NULL;
 	}
 
-	if (!(field = kmalloc(sizeof(struct hid_field) + usages * sizeof(struct hid_usage)
-		+ values * sizeof(unsigned), GFP_KERNEL))) return NULL;
-
-	memset(field, 0, sizeof(struct hid_field) + usages * sizeof(struct hid_usage)
-		+ values * sizeof(unsigned));
+	sz = sizeof(struct hid_field) + usages * sizeof(struct hid_usage) +
+			values * sizeof(unsigned);
+	if ((field = kmalloc(sz, GFP_KERNEL)) == NULL)
+		return NULL;
+	memset(field, 0, sz);
 
 	field->index = report->maxfield++;
 	report->field[field->index] = field;


-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/