linux-kernel - Re: [RFC][PATCH 3/6] SLIM main patch

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: <1152907301.23584.38.camel@localhost.localdomain>
Date:	Fri, 14 Jul 2006 13:01:41 -0700
From:	Kylene Jo Hall <kjhall@...ibm.com>
To:	Dave Hansen <haveblue@...ibm.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	LSM ML <linux-security-module@...r.kernel.org>,
	Dave Safford <safford@...ibm.com>,
	Mimi Zohar <zohar@...ibm.com>, Serge Hallyn <sergeh@...ibm.com>
Subject: Re: [RFC][PATCH 3/6] SLIM main patch

On Fri, 2006-07-14 at 11:27 -0700, Dave Hansen wrote:

> > +static enum slm_iac_level set_iac(char *token)
> > +{
> > +	int iac;
> > +
> > +	if (memcmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
> > +		return SLM_IAC_EXEMPT;
> > +	else {
> 
> Might as well add brackets here.  Or, just kill the else{} block and
> pull the code back to the lower indenting level.  The else is really
> unnecessary because of the return;
> 
> > +		for (iac = 0; iac < sizeof(slm_iac_str) / sizeof(char *); iac++) {
> > +			if (memcmp(token, slm_iac_str[iac],
> > +				   strlen(slm_iac_str[iac])) == 0)
> > +				return iac;
> 
> Why not use strcmp?
> 
> > +static enum slm_sac_level set_sac(char *token)
> > +{
> > +	int sac;
> > +
> > +	if (memcmp(token, EXEMPT_STR, strlen(EXEMPT_STR)) == 0)
> > +		return SLM_SAC_EXEMPT;
> > +	else {
> > +		for (sac = 0; sac < sizeof(slm_sac_str) / sizeof(char *); sac++) {
> > +			if (memcmp(token, slm_sac_str[sac],
> > +				   strlen(slm_sac_str[sac])) == 0)
> > +				return sac;
> > +		}
> > +	}
> > +	return SLM_SAC_ERROR;
> > +}
> 
> This function looks awfully similar :).  Can you just pass that array in
> as an argument, and get rid of one of the functions?
> 
On closer look combining these would require collapsing them into one
enum or returning int and doing a bunch of casting.  Kind of seems to
void the point of using an enum.  Thus I propose leaving them as is,
okay?


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