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 <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
