Re: Patch for setting the keyboard LEDs state

[Marco Gerards]

Marcus Brinkmann <Marcus.Brinkmann@ruhr-uni-bochum.de> writes:

> On Thu, Jan 29, 2004 at 02:09:46AM +0100, Marco Gerards wrote:
> > Hi,
> > 
> > I put a patch (#2543) on savannah to change the keyboard LEDs state in
> > GNUMach.  This can be used with device_set_status.  I've also put a
> > patch on savannah so the keyboard driver (pc_kbd) of the Hurd console
> > can use this.
> 
> Please put all variables (data_cnt, led) into the else{} block.  You then
> need to copy the led variable into the if{} block.  Even though their values
> seem to agree by chance, let's not rely on this, just duplicate it so that
> GNU Mach 1 and 2 are two independent chunks of code.  This makes it easier
> to see what is going on.

Sure, I will make this change.

> Also put the code to change the LED for GNUMach into an #ifdef KDSETLEDS,
> so that compiles with old headers don't fail.

I defined KDSETLED macro in pc_kbd.c (just like you did for the other
macros).  This makes compiling independent of the the Mach version.
Just like this code works on every Mach version (I forgot to mention
that in my previous mail).

So AFAIK I don't have to change anything here.
 
> Second, you have your keyboard fix in this patch, so take care not to check
> it in accidentially.

It is not accidental.  I think this patch is more appropriate here.  I
assume it can take a while before someone reads my autoconf related
patch while this small change is really small and can be checked in
some minutes.

So I would really like it if I could apply it all in once. :)

> Third, in your acupdate diff I spotted:
> +AC_CHECK_TOOL(LD, ld)AC_SUBST(LD_FLAGS)
> 
> Watch out for such stuff.  I always go over the diff before posting it
> somewhere for spurious changes like this.

Urgh, I really wonder why I missed this, sorry.

> If you have done the above, you can check in the LED patches.

Before doing that I will send the patch to bug-hurd first because both
Alfred and Roland asked me.

>From now on I will put patches on savannah and send them to the
mailinglist.

Thanks,
Marco