linux-kernel - Re: [PATCH] sysfs: kill an extra put in sysfs_create

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:	Thu, 19 Jul 2007 00:43:38 +0900
From:	Tejun Heo <htejun@...il.com>
To:	Satyam Sharma <satyam.sharma@...il.com>
CC:	Gabriel C <nix.or.die@...glemail.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Christoph Lameter <clameter@....com>, gregkh@...e.de,
	miles.lane@...il.com
Subject: Re: [PATCH] sysfs: kill an extra put in sysfs_create_link() failure
 path

Satyam Sharma wrote:
>> Well, I dunno.  Probably my taste just sucks.  Please feel free to
>> submit patches and/or suggest better ideas.
> 
> OK, for example:
> 
> sysfs_find_dirent() -- to check for -EEXIST -- should be called
> *before* we create the new dentry for the to-be-created symlink
> in the first place. [ It's weird to grab a reference on the target
> for ourselves (and in fact even allocate the new dirent for the
> to-be-created symlink) and /then/ check for erroneous usage,
> and then go about undoing all that we should never have done
> at all. ] So this test could, and should, be made earlier, IMHO.

Locking.  Otherwise, why would the code look like that in the first place?

> And some similar others ... so attached (sorry, Gmail web
> interface) please find an attempt to make sysfs_create_link look
> a trifle more like what it should look like, IMHO. The code cleanup
> also leads to fewer LOC, smaller kernel image (lesser by 308 bytes),
> and even speeding up the no-error common case of this function,
> apart from the obvious readability benefits ... it's diffed on _top_ of
> your bugfix here, but not the other patch. [ Compile-tested only. ]

Compounded if-else vs. flattened if () with common error path is pretty
much matter of being accustomed to.  I prefer the latter because it
scales better (less nesting and less need for extra intelligence as
error case grows).  As I'm already used to it, it's also easier on my
eyes.  So, unless you have more to offer, I'm not really sure whether
the patch improves the situation noticeably.

-- 
tejun
-
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/