On Friday, April 2, 2021, Corey Minyard <miny...@acm.org> wrote:
> On Tue, Mar 30, 2021 at 09:16:44PM +0300, Andy Shevchenko wrote:
> > Instead of twice repeat the constant literals, introduce
> > panic_event_str array. It allows to simplify the code with
> > help of match_string() API.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> > ---
> > drivers/char/ipmi/ipmi_msghandler.c | 49 ++++++++++-------------------
> > 1 file changed, 17 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c
> b/drivers/char/ipmi/ipmi_msghandler.c
> > index f19f0f967e28..c7d37366d7bb 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -52,8 +52,12 @@ static bool drvregistered;
> > enum ipmi_panic_event_op {
> > IPMI_SEND_PANIC_EVENT_NONE,
> > IPMI_SEND_PANIC_EVENT,
> > - IPMI_SEND_PANIC_EVENT_STRING
> > + IPMI_SEND_PANIC_EVENT_STRING,
> > + IPMI_SEND_PANIC_EVENT_MAX
> > };
>
> This is a nice change. Can you add a comment here so that readers know
> that the above enum and the following array are tied numerically?
Sure. I also add a prefix.
>
> -corey
>
> > +
> > +static const char *const panic_event_str[] = { "none", "event",
> "string", NULL };
> > +
> > #ifdef CONFIG_IPMI_PANIC_STRING
> > #define IPMI_PANIC_DEFAULT IPMI_SEND_PANIC_EVENT_STRING
> > #elif defined(CONFIG_IPMI_PANIC_EVENT)
> > @@ -68,46 +72,27 @@ static int panic_op_write_handler(const char *val,
> > const struct kernel_param *kp)
> > {
> > char valcp[16];
> > - char *s;
> > -
> > - strncpy(valcp, val, 15);
> > - valcp[15] = '\0';
> > + int e;
> >
> > - s = strstrip(valcp);
> > -
> > - if (strcmp(s, "none") == 0)
> > - ipmi_send_panic_event = IPMI_SEND_PANIC_EVENT_NONE;
> > - else if (strcmp(s, "event") == 0)
> > - ipmi_send_panic_event = IPMI_SEND_PANIC_EVENT;
> > - else if (strcmp(s, "string") == 0)
> > - ipmi_send_panic_event = IPMI_SEND_PANIC_EVENT_STRING;
> > - else
> > - return -EINVAL;
> > + strscpy(valcp, val, sizeof(valcp));
> > + e = match_string(panic_event_str, -1, strstrip(valcp));
> > + if (e < 0)
> > + return e;
> >
> > + ipmi_send_panic_event = e;
> > return 0;
> > }
> >
> > static int panic_op_read_handler(char *buffer, const struct
> kernel_param *kp)
> > {
> > - switch (ipmi_send_panic_event) {
> > - case IPMI_SEND_PANIC_EVENT_NONE:
> > - strcpy(buffer, "none\n");
> > - break;
> > -
> > - case IPMI_SEND_PANIC_EVENT:
> > - strcpy(buffer, "event\n");
> > - break;
> > -
> > - case IPMI_SEND_PANIC_EVENT_STRING:
> > - strcpy(buffer, "string\n");
> > - break;
> > + const char *event_str;
> >
> > - default:
> > - strcpy(buffer, "???\n");
> > - break;
> > - }
> > + if (ipmi_send_panic_event >= IPMI_SEND_PANIC_EVENT_MAX)
> > + event_str = "???";
> > + else
> > + event_str = panic_event_str[ipmi_send_panic_event];
> >
> > - return strlen(buffer);
> > + return sprintf(buffer, "%s\n", event_str);
> > }
> >
> > static const struct kernel_param_ops panic_op_ops = {
> > --
> > 2.30.2
> >
>
--
With Best Regards,
Andy Shevchenko
_______________________________________________
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
