[LEDE-DEV] [PATCH] procd: Remove unnecessary memset calls.

[Paul Oranje]

Thanks, also for the link (https://vorpus.org/blog/why-does-calloc-exist/). This article made me aware that using calloc() may be wise.

For security reasons: calloc avoids possible arrhythmic overflows when the allocation size for malloc is the product of two numbers.
And because it may offer real performance advantages with big allocations: calloc() can make good use of the VM syscalls, whereas memset() needs to initialise all allocated memory at once which requires that concerned memory is paged in, with calloc() memory is allocated sparsely.

The article makes a good argument for using malloc(), but still one should agree with Arjen de Korte, that no changes should be made without necessity. When I was an active C programmer I was called "de snoeier" (Dutch for pruner) and did, to mine and others dismay, introduce errors. Not making errors, even with seemingly trivial changes, is hard.
For new code this disadvantage of changing working code is absent ...

Paul


> Op 8 nov. 2017, om 19:59 heeft rosenp at gmail.com het volgende geschreven:
> 
> On Wed, 2017-11-08 at 11:57 +0100, Paul Oranje wrote:
>> Both memset() and calloc() have highly optimised implementations, so
>> the expected gains with this patch for the allocation of zeroed
>> memory will be small at best. As this patch does not fix a bug: why
>> is the change "needed" ?
>> 
> Style changes are strictly speaking not "needed".
>> Just curiosity, bye,
>> Paul
>> 
>>> Op 7 nov. 2017, om 21:05 heeft Rosen Penev <rosenp at gmail.com> het
>>> volgende geschreven:
>>> 
>>> Changes allocation to calloc and {} as needed.
>>> 
>>> Signed-off-by: Rosen Penev <rosenp at gmail.com>
>>> ---
>>> inittab.c      | 6 ++----
>>> plug/hotplug.c | 7 ++-----
>>> 2 files changed, 4 insertions(+), 9 deletions(-)
>>> 
>>> diff --git a/inittab.c b/inittab.c
>>> index 21172f7..c27c324 100644
>>> --- a/inittab.c
>>> +++ b/inittab.c
>>> @@ -284,8 +284,7 @@ void procd_inittab(void)
>>> 
>>> 	regcomp(&pat_inittab, "([a-zA-Z0-9]*):([a-zA-Z0-9]*):([a-zA-Z0-
>>> 9]*):(.*)", REG_EXTENDED);
>>> 	line = malloc(LINE_LEN);
>>> -	a = malloc(sizeof(struct init_action));
>>> -	memset(a, 0, sizeof(struct init_action));
>>> +	a = calloc(1, sizeof(struct init_action));
>>> 
>>> 	while (fgets(line, LINE_LEN, fp)) {
>>> 		char *tags[TAG_PROCESS + 1];
>>> @@ -322,8 +321,7 @@ void procd_inittab(void)
>>> 		if (add_action(a, tags[TAG_ACTION]))
>>> 			continue;
>>> 		line = malloc(LINE_LEN);
>>> -		a = malloc(sizeof(struct init_action));
>>> -		memset(a, 0, sizeof(struct init_action));
>>> +		a = calloc(1, sizeof(struct init_action));
>>> 	}
>>> 
>>> 	fclose(fp);
>>> diff --git a/plug/hotplug.c b/plug/hotplug.c
>>> index 49c177f..6e55f67 100644
>>> --- a/plug/hotplug.c
>>> +++ b/plug/hotplug.c
>>> @@ -434,12 +434,10 @@ static void handle_button_complete(struct
>>> blob_attr *msg, struct blob_attr *data
>>> 	if (!name)
>>> 		return;
>>> 
>>> -	b = malloc(sizeof(*b));
>>> +	b = calloc(1, sizeof(*b));
>>> 	if (!b)
>>> 		return;
>>> 
>>> -	memset(b, 0, sizeof(*b));
>>> -
>>> 	b->data = malloc(blob_pad_len(data));
>>> 	b->name = strdup(name);
>>> 	b->seen = timeout;
>>> @@ -584,11 +582,10 @@ void hotplug_last_event(uloop_timeout_handler
>>> handler)
>>> 
>>> void hotplug(char *rules)
>>> {
>>> -	struct sockaddr_nl nls;
>>> +	struct sockaddr_nl nls = {};
>>> 	int nlbufsize = 512 * 1024;
>>> 
>>> 	rule_file = strdup(rules);
>>> -	memset(&nls,0,sizeof(struct sockaddr_nl));
>>> 	nls.nl_family = AF_NETLINK;
>>> 	nls.nl_pid = getpid();
>>> 	nls.nl_groups = -1;
>>> -- 
>>> 2.13.6
>>> 
>>> 
>>> _______________________________________________
>>> Lede-dev mailing list
>>> Lede-dev at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/lede-dev
>> 
>>