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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
