lkml.org 
[lkml]   [2005]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: RFC: turn kmalloc+memset(,0,) into kcalloc
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/

\
 
 \ /
  Last update: 2005-04-09 16:24    [W:0.090 / U:0.544 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site