linux-kernel - Re: [PATCH v3 1/2] Staging: lustre: Refactor the function interval_erase_color() in /lustre/ldlm/interval

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] [day] [month] [year] [list]

Date:	Sat, 11 Jan 2014 12:33:58 -0800
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Monam Agarwal <monamagarwal123@...il.com>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	devel@...verdev.osuosl.org, andreas.dilger@...el.com,
	peter.p.waskiewicz.jr@...el.com, linux-kernel@...r.kernel.org,
	Rashika Kheria <rashika.kheria@...il.com>
Subject: Re: [PATCH v3 1/2] Staging: lustre: Refactor the function
 interval_erase_color() in /lustre/ldlm/interval_tree.c

On Sat, Jan 11, 2014 at 05:14:35PM +0530, Monam Agarwal wrote:
> On Sat, Jan 11, 2014 at 5:09 PM, Dan Carpenter <dan.carpenter@...cle.com> wrote:
> > On Sat, Jan 11, 2014 at 04:56:44PM +0530, Monam Agarwal wrote:
> >> I took n as a flag to decide whether parent->in_left == node is true
> >> or not in the called function.
> >
> > So "n" stands for "node"?
> >
> >> Should I use some other name for the flag.
> >
> >
> > Yes.
> >
> 
> Will "flag" be a suitable name?

Ick, no.  You don't want a "flag" to have to determine what the logic is
for a given function.  That just causes confusion and makes things
really hard to read and understand over time.

This whole function looks like a red/black tree, or something like that.
Shouldn't we just be using the in-kernel implementation of this?  And if
not, then you really need to get the feedback of the code's original
authors as you might be changing the algorithm in ways that could cause
big problems.

thanks,

greg k-h
--
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/