linux-kernel - Re: [Xen-devel] [PATCH v2] Input: xen-kbdfront

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: <CAKf6xpuogcR96nq1OLmfk2=7URC_+d8aFZ-cWTWEYPNeQh=oWg@mail.gmail.com>
Date:   Mon, 23 Apr 2018 14:40:08 -0400
From:   Jason Andryuk <jandryuk@...il.com>
To:     Oleksandr Andrushchenko <andr2000@...il.com>
Cc:     xen-devel@...ts.xenproject.org, linux-input@...r.kernel.org,
        open list <linux-kernel@...r.kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Juergen Gross <jgross@...e.com>, lyan@...e.com,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        andrii_chepurnyi@...m.com,
        Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
Subject: Re: [Xen-devel] [PATCH v2] Input: xen-kbdfront - allow better
 run-time configuration

Hi, Oleksandr.

On Thu, Apr 19, 2018 at 9:39 AM, Oleksandr Andrushchenko
<andr2000@...il.com> wrote:

<snip>

> @@ -241,60 +242,84 @@ static int xenkbd_probe(struct xenbus_device *dev,
>         }
>
>         /* keyboard */
> -       kbd = input_allocate_device();
> -       if (!kbd)
> -               goto error_nomem;
> -       kbd->name = "Xen Virtual Keyboard";
> -       kbd->phys = info->phys;
> -       kbd->id.bustype = BUS_PCI;
> -       kbd->id.vendor = 0x5853;
> -       kbd->id.product = 0xffff;
> -
> -       __set_bit(EV_KEY, kbd->evbit);
> -       for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> -               __set_bit(i, kbd->keybit);
> -       for (i = KEY_OK; i < KEY_MAX; i++)
> -               __set_bit(i, kbd->keybit);
> -
> -       ret = input_register_device(kbd);
> -       if (ret) {
> -               input_free_device(kbd);
> -               xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> -               goto error;
> +       if (!no_kbd_dev) {

My earlier suggestion on the option name was aimed at replacing the above with:
   if (kbd_dev) {

I find it easier to read then the double negative !no_kbd_dev.  But
it's only used once, so it's not a big deal either way.

<snip>

>
> -       __set_bit(EV_KEY, ptr->evbit);
> -       for (i = BTN_LEFT; i <= BTN_TASK; i++)
> -               __set_bit(i, ptr->keybit);
> +               ptr = input_allocate_device();
> +               if (!ptr)
> +                       goto error_nomem;
> +               ptr->name = "Xen Virtual Pointer";
> +               ptr->phys = info->phys;
> +               ptr->id.bustype = BUS_PCI;
> +               ptr->id.vendor = 0x5853;
> +               ptr->id.product = 0xfffe;
> +
> +               if (abs) {
> +                       __set_bit(EV_ABS, ptr->evbit);
> +                       input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0);
> +                       input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0);

Just noticed these lines now exceed 80 columns.

Otherwise it's just code motion and fine by me.

Regards,
Jason