On Fri, 1 Dec 2006 22:36:55 -0600
Corey Minyard <[EMAIL PROTECTED]> wrote:

> 
> Add the ability to hot add and remove interfaces in the ipmi_si
> driver.  Any users who have the device open will get errors if they
> try to send a message.
> 
> ...
>
> +static int ipmi_strcasecmp(const char *s1, const char *s2)
> +{
> +     while (*s1 || *s2) {
> +             if (!*s1)
> +                     return -1;
> +             if (!*s2)
> +                     return 1;
> +             if (*s1 != *s2)
> +                     return *s1 - *s2;
> +             s1++;
> +             s2++;
> +     }
> +     return 0;
> +}

Please put a newline between functions.

> +static int parse_str(struct hotmod_vals *v, int *val, char *name, char 
> **curr)
> +{
> +     char *s;
> +     int  i;
> +
> +     s = strchr(*curr, ',');
> +     if (!s) {
> +             printk(KERN_WARNING PFX "No hotmod %s given.\n", name);
> +             return -EINVAL;
> +     }
> +     *s = '\0';
> +     s++;
> +     for (i = 0; hotmod_ops[i].name; i++) {
> +             if (ipmi_strcasecmp(*curr, v[i].name) == 0) {
> +                     *val = v[i].val;
> +                     *curr = s;
> +                     return 0;
> +             }
> +     }
> +
> +     printk(KERN_WARNING PFX "Invalid hotmod %s '%s'\n", name, *curr);
> +     return -EINVAL;
> +}

Does this driver really need case-insensitive string comparision?

<greps the tree for casecmp, sighs>

That doesn't look like a case-insensitive string compare to me.  What's up?

> ...
>
> +             while (s) {
> +                     curr = s;
> +                     s = strchr(curr, ',');
> +                     if (s) {
> +                             *s = '\0';
> +                             s++;
> +                     }
> +                     o = strchr(curr, '=');
> +                     if (o) {
> +                             *o = '\0';
> +                             o++;
> +                     }
> +#define HOTMOD_INT_OPT(name, val) \
> +                     if (ipmi_strcasecmp(curr, name) == 0) {         \
> +                             if (!o) {                               \
> +                                     printk(KERN_WARNING PFX         \
> +                                            "No option given for '%s'\n", \
> +                                             curr);                  \
> +                                     goto out;                       \
> +                             }                                       \
> +                             val = simple_strtoul(o, &n, 0);         \
> +                             if ((*n != '\0') || (*o == '\0')) {     \
> +                                     printk(KERN_WARNING PFX         \
> +                                            "Bad option given for '%s'\n", \
> +                                            curr);                   \
> +                                     goto out;                       \
> +                             }                                       \
> +                     }
> +
> +                     HOTMOD_INT_OPT("rsp", regspacing)
> +                     else HOTMOD_INT_OPT("rsi", regsize)
> +                     else HOTMOD_INT_OPT("rsh", regshift)
> +                     else HOTMOD_INT_OPT("irq", irq)
> +                     else HOTMOD_INT_OPT("ipmb", ipmb)

oh yuk.  Can't this be done via a function or something?


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

Reply via email to