netdev - Re: [Linux-zigbee-devel] [PATCH] mrf24j40: seperate mrf24j40 h/w init and add checkings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 23 Apr 2014 11:36:34 -0300
From:	Werner Almesberger <werner@...esberger.net>
To:	Alan Ott <alan@...nal11.us>
Cc:	Varka Bhadram <varkabhadram@...il.com>, alex.aring@...il.com,
	Varka Bhadram <varkab@...c.in>, netdev@...r.kernel.org,
	linux-zigbee-devel@...ts.sourceforge.net, davem@...emloft.net
Subject: Re: [Linux-zigbee-devel] [PATCH] mrf24j40: seperate mrf24j40 h/w
 init and add checkings

Alan Ott wrote:
> 3. The code to check for it just adds a lot of bloat without much 
> measurable benefit.

As a very general note, if - in any C program, not just the kernel
- you have too many error checks for comfort, you may want to
consider keeping a cumulative error status (e.g., in this case,
struct mrf24j40), and just check that. Similar to ferror in stdio.

Something like


static int my_check_and_clear_rc(struct foo *foo)
{
	int rc;

	rc = foo->rc;
	foo->rc = 0;
	return rc;
}


static int my_operation(struct foo *foo, int arg)
{
	int rc;

	/* don't make it worse - optional */
	if (foo->rc)
		return foo->rc;

	rc = really_do_my_operation(foo, arg);
	if (rc < 0 && !foo->rc)
		foo->rc = rc;

	return foo->rc ? foo->rc : rc;
}


Then the phalanx of tedious checks shrinks to

	/* make sure foo->rc is initialized to 0 */

	my_operation(foo, 1);
	my_operation(foo, 2);
	...
	my_operation(foo, 1000);

	rc = my_check_and_clear_rc(foo);
	if (rc) {
		complain("something terribly wrong");
		...
	}
	...

You can easily extend this to also record line numbers, file
names, and such, if necessary. Add atomic/locking as needed.

- Werner
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html