On Thursday 09 June 2016 13:27:21 Michał Kępień wrote:
> > > - case 0x10:
> > > - /* Keys pressed */
> > > + case 0x0010:
> > > + /* Sequence of keys pressed */
> > >
> > > for (i = 2; i < len; ++i)
> > >
> > > - dell_wmi_process_key(buffer_entry[i]);
> > > + dell_wmi_process_key(0x0010, buffer_entry[i]);
> > >
> > > break;
> > >
> > > - case 0x11:
> > > - for (i = 2; i < len; ++i) {
> > > - switch (buffer_entry[i]) {
> > > - case 0xfff0:
> > > - /* Battery unplugged */
> > > - pr_debug("Battery unplugged\n");
> > > - break;
> > > - case 0xfff1:
> > > - /* Battery inserted */
> > > - pr_debug("Battery inserted\n");
> > > - break;
> > > - case 0x01e1:
> > > - case 0x02ea:
> > > - case 0x02eb:
> > > - case 0x02ec:
> > > - case 0x02f6:
> > > - /* Keyboard backlight level changed */
> > > - pr_debug("Keyboard backlight level "
> > > - "changed\n");
> > > - break;
> > > - default:
> > > - /* Unknown event */
> > > - pr_info("Unknown WMI event type 0x11: "
> > > - "0x%x\n", (int)buffer_entry[i]);
> > > - break;
> > > - }
> > > - }
> > > + case 0x0011:
> > > + /* Sequence of events occurred */
> > > + for (i = 2; i < len; ++i)
> > > + dell_wmi_process_key(0x0011, buffer_entry[i]);
> >
> > Since this is identical to case 0x010, let's avoid the duplication
> > of code and
> >
> > handle this with a fall-through, like:
> > case 0x0010:
> > case 0x0011:
> > /* Sequence of events occurred */
> > for (i = 2; i < len; ++i)
> >
> > dell_wmi_process_key(buffer_entry[1],
> > buffer_entry[i]);
>
> I believe it was Pali's intention to make a distinction between keys
> being pressed (0x0010) and other events occuring (0x0011), so perhaps
> a comment after the first case label could be useful?
>
> case 0x0010:
> /* Sequence of keys pressed; fall through */
> case 0x0011:
> /* Sequence of events occurred */
> for (i = 2; i < len; ++i)
> dell_wmi_process_key(buffer_entry[1], buffer_entry[i]);
>
> I'll leave it to Pali to decide, I'm just throwing in my two cents.Yes, that is truth, to visually distinguish between events and keys. -- Pali Rohár [email protected]
signature.asc
Description: This is a digitally signed message part.
