[PATCH keyboard 2/4] Cleanup set the value of CustomKeycodes

[Alexandr Shadchin]

On Thu, Feb 17, 2011 at 08:32:31AM +1000, Peter Hutterer wrote:
> On Mon, Feb 14, 2011 at 12:45:39PM +0500, Alexandr Shadchin wrote:
> > On Mon, Feb 14, 2011 at 10:11:02AM +1000, Peter Hutterer wrote:
> > > On Fri, Feb 11, 2011 at 03:32:58PM +0500, Alexandr Shadchin wrote:
> > > > Also, do not print values of variables CustomKeycodes twice.
> > > > 
> > > > Signed-off-by: Alexandr Shadchin <Alexandr.Shadchin at gmail.com>
> > > > ---
> > > >  src/bsd_kbd.c  |    1 -
> > > >  src/hurd_kbd.c |    1 -
> > > >  src/kbd.c      |   13 ++-----------
> > > >  src/sun_kbd.c  |    2 --
> > > >  4 files changed, 2 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/src/bsd_kbd.c b/src/bsd_kbd.c
> > > > index 127b6ab..1e432fd 100644
> > > > --- a/src/bsd_kbd.c
> > > > +++ b/src/bsd_kbd.c
> > > > @@ -444,7 +444,6 @@ xf86OSKbdPreInit(InputInfoPtr pInfo)
> > > >      pKbd->RemapScanCode = NULL;
> > > >  
> > > >      pKbd->OpenKeyboard = OpenKeyboard;
> > > > -    pKbd->CustomKeycodes = FALSE;
> > > >  
> > > >      pKbd->private = calloc(sizeof(BsdKbdPrivRec), 1);
> > > >      if (pKbd->private == NULL) {
> > > > diff --git a/src/hurd_kbd.c b/src/hurd_kbd.c
> > > > index dde5fbb..8c0cd60 100644
> > > > --- a/src/hurd_kbd.c
> > > > +++ b/src/hurd_kbd.c
> > > > @@ -158,7 +158,6 @@ xf86OSKbdPreInit(InputInfoPtr pInfo)
> > > >      pKbd->KbdGetMapping = KbdGetMapping;
> > > >      pKbd->RemapScanCode = ATScancode;
> > > >      pKbd->OpenKeyboard  = OpenKeyboard;
> > > > -    pKbd->CustomKeycodes = FALSE;
> > > >      pKbd->private       = NULL;
> > > >      pInfo->read_input   = ReadInput;
> > > >      return TRUE;
> > > > diff --git a/src/kbd.c b/src/kbd.c
> > > > index f4b51ff..f5cecbb 100644
> > > > --- a/src/kbd.c
> > > > +++ b/src/kbd.c
> > > > @@ -151,7 +151,6 @@ KbdPreInit(InputDriverPtr drv, InputInfoPtr pInfo, int flags)
> > > >  #endif
> > > >  {
> > > >      KbdDevPtr pKbd;
> > > > -    MessageType from = X_DEFAULT;
> > > >      char *s;
> > > >      const char **defaults;
> > > >      int rc = Success;
> > > > @@ -220,16 +219,8 @@ KbdPreInit(InputDriverPtr drv, InputInfoPtr pInfo, int flags)
> > > >      xkb_variant = xf86SetStrOption(pInfo->options, "XkbVariant", NULL);
> > > >      xkb_options = xf86SetStrOption(pInfo->options, "XkbOptions", NULL);
> > > >  
> > > > -  pKbd->CustomKeycodes = FALSE;
> > > > -  from = X_DEFAULT; 
> > > > -  if (xf86FindOption(pInfo->options, "CustomKeycodes")) {
> > > > -      pKbd->CustomKeycodes = xf86SetBoolOption(pInfo->options, "CustomKeycodes",
> > > > -                                               pKbd->CustomKeycodes);
> > > > -     from = X_CONFIG;
> > > > -  }
> > > > -
> > > > -  xf86Msg(from, "%s: CustomKeycodes %s\n",
> > > > -               pInfo->name, pKbd->CustomKeycodes ? "enabled" : "disabled");
> > > > +    pKbd->CustomKeycodes = xf86SetBoolOption(pInfo->options, "CustomKeycodes",
> > > > +                                             NULL);
> > > 
> > > NULL is a rather unusual choice for a boolean. ACK otherwise though.
> > > 
> > > Cheers,
> > >   Peter
> > 
> > Before calling xf86SetBoolOption(pInfo->options, "CustomKeycodes", NULL)
> > we've done xf86CollectInputOptions(pInfo, defaults).
> > 
> > defaults = kbdDefaults or kbd98Defaults
> > 
> > static const char *kbdDefaults[] = {
> >     "Protocol",         "standard",
> >     "XkbRules",         "base",
> >     "XkbModel",         "pc105",
> >     "XkbLayout",        "us",
> >     "CustomKeycodes",   "off", <-------- set by default
> >     NULL
> > };
> > 
> > static const char *kbd98Defaults[] = {
> >     "Protocol",         "standard",
> >     "XkbRules",         "xfree98",
> >     "XkbModel",         "pc98",
> >     "XkbLayout",        "jp",
> >     "CustomKeycodes",   "off", <-------- set by default
> >     NULL
> > };
> > 
> > Thus the value of "CustomKeycodes" when calling xf86SetBoolOption will already be set
> > (by default or from the config) and xf86SetBoolOption can not return NULL.
> > 
> > Indeed replacement of NULL will add clarity, but functionally not change anything.
> > What is your opinion on this issue?
> 
> sorry, swamped. my comment was merely stating that instead of NULL we should
> use FALSE. no functional change, but makes more sense when reading the code.
> 
> Cheers,
>   Peter

I totally agree. I did not immediately realize :-)

-- 
Alexandr Shadchin