linux-kernel - Re: [V9fs-developer] [PATCH 1/8] fs/9p: Deletion of unnecessary checks before the function call "p9_client

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 6 Jan 2015 16:02:02 +0100
From:	Dominique Martinet <dominique.martinet@....fr>
To:	SF Markus Elfring <elfring@...rs.sourceforge.net>
CC:	Julia Lawall <julia.lawall@...6.fr>,
	Latchesar Ionkov <lucho@...kov.net>,
	Eric Van Hensbergen <ericvh@...il.com>,
	<kernel-janitors@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Ron Minnich <rminnich@...dia.gov>,
	<v9fs-developer@...ts.sourceforge.net>,
	Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [V9fs-developer] [PATCH 1/8] fs/9p: Deletion of unnecessary
 checks before the function call "p9_client_clunk"

SF Markus Elfring wrote on Tue, Jan 06, 2015 at 03:34:08PM +0100:
> > Actually just seen that this precise example is fixed along with more
> > similar code paths in subsequents (!) patchs of the set.
> 
> Do you refer to my update suggestions with a subject like "One function
> call less in v9fs_…" here?

Yes. Looking at the patchset as a whole (and under the assumption that
calling p9_client_clunk with NULL is harmful), your patchset first
introduces many such calls then proceeds onto fixing them.

Thinking a bit more, I think that instead of just changing the order of
patches the cleanest way would be to submit this as "reworking jump
labels, making NULL checks useless in the process".
Still only looking at v9fs_create, with your patch#2 reflowing,
p9_client_clunk can't be called with NULL unless a bug happened (either
p9_client_fcreate or p9_client_walk returning non-error but leaving ofid
or fid NULL), so it'd actually make sense to remove the quiet check in
favor of the warn/stack dump check at this point.

Is what I'm saying making sense?

> > It could actually be interesting to see if we could get all such
> > paths "fixed".
> 
> Would you like to see any more specific source code clean-up?
> Which kind of fine-tuning have you got in mind?

I was really just thinking that if the code flow is thought out the if's
can be removed harmlessly, but that it actually needs to be done the
other way around.
I honestly am not involved enough in kernel coding style changes or 9p
code as a whole to suggest anything else, sorry :)

Thanks,
-- 
Dominique Martinet,
CEA
--
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/