linux-kernel - Re: [PATCH] include/linux/slab.h: new KFREE() macro.

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]

Message-Id: <1168212133.2744.17.camel@dsl081-166-245.sea1.dsl.speakeasy.net>
Date:	Sun, 07 Jan 2007 15:22:13 -0800
From:	Vadim Lobanov <vlobanov@...akeasy.net>
To:	Amit Choudhary <amit2030@...oo.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] include/linux/slab.h: new KFREE() macro.

On Sun, 2007-01-07 at 14:43 -0800, Amit Choudhary wrote:
> Any strong reason why not? x has some value that does not make sense and can create only problems.
> And as I explained, it can result in longer code too. So, why keep this value around. Why not
> re-initialize it to NULL.

Because it looks really STRANGE(tm). Consider the following function,
which is essentially what you're proposing in macro-ized form:
	void foobar(void)
	{
		void *ptr;

		ptr = kmalloc(...);
		// actual work here
		kfree(ptr);
		ptr = NULL;
	}
Reading code like that makes me say "wtf?", simply because 'ptr' is not
used thereafter, so setting it to NULL is both pointless and confusing
(it looks out-of-place, and therefore makes me wonder if there's
something stupidly tricky going on).

Also, arguably, your demonstration of why the lack of the proposed
KFREE() macro results in longer code is invalid. Whereas you wrote:
	pointer *arr_x[size_x];
	pointer *arr_y[size_y];
	pointer *arr_z[size_z];
That really should have been:
	pointer *arr[size_x + size_y + size_z];
or:
	pointer **arr[3] = { arr_x, arr_y, arr_z };
In which case, the you only need one path in the function to handle
allocation failures, rather than the three that you were arguing for.

> If x should not be re-initialized to NULL, then by the same logic, we should not even initialize
> local variables. And all of us know that local variables should be initialized.

That's some strange and confused logic then. Here's my stab at the same
logical premise: "Using uninitialized values is bad." Notice how that,
in and of itself, makes no statements regarding freed pointers, since
the intent is not to use them after they've been freed anyway.

-- Vadim Lobanov


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/