Hi,
at_keyboard's checkkey() implementation is buggy. It will flush the
controller's input buffer, but checkkey()'s semantics require that it's
not flushed. This results in some keys being lost when using at_keyboard.
A proper fix that adjusts to this API is complicated. I implemented one [1],
only to realize afterwards that maybe this mess isn't really necessary.
A quick check at grub_checkkey() users reveals that none of them care about
the actual key, even if grub_checkkey returns it. They just want to know
whether there's a key available for read at all. If we removed this
constraint from the checkkey() semantics, perhaps at_keyboard can get a
checkkey() with less hassle.
However, when at_keyboard reads an event, it needs to determine whether it's
a MAKE (key hit) or BREAK (key unhit) event. Which AFAICT can't be done
without reading KEYBOARD_REG_DATA. But maybe I'm missing something.
Any bright ideas?
[1] See attachment. This made me implement queues as well, perhaps some of
this can be reused.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
Index: conf/i386.rmk
===================================================================
--- conf/i386.rmk (revision 2607)
+++ conf/i386.rmk (working copy)
@@ -6,7 +6,7 @@ cpuid_mod_CFLAGS = $(COMMON_CFLAGS)
cpuid_mod_LDFLAGS = $(COMMON_LDFLAGS)
pkglib_MODULES += at_keyboard.mod
-at_keyboard_mod_SOURCES = term/i386/pc/at_keyboard.c
+at_keyboard_mod_SOURCES = term/i386/pc/at_keyboard.c kern/queue.c
at_keyboard_mod_CFLAGS = $(COMMON_CFLAGS)
at_keyboard_mod_LDFLAGS = $(COMMON_LDFLAGS)
Index: kern/queue.c
===================================================================
--- kern/queue.c (revision 0)
+++ kern/queue.c (revision 0)
@@ -0,0 +1,53 @@
+/* queue.c - grub queue functions */
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/queue.h>
+#include <grub/misc.h>
+#include <grub/mm.h>
+
+void
+grub_queue_push (grub_queue_t queue, grub_queue_item_t item)
+{
+ item->next = queue->head;
+ item->prev = NULL;
+ if (queue->head)
+ queue->head->prev = item;
+ queue->head = item;
+
+ if (! queue->tail)
+ /* Empty queue becomes non-empty. */
+ queue->tail = item;
+}
+
+void *
+grub_queue_pop (grub_queue_t queue)
+{
+ grub_queue_item_t item;
+
+ item = queue->tail;
+ if (queue->tail)
+ queue->tail = item->prev;
+ if (queue->tail)
+ queue->tail->next = NULL;
+ else
+ /* Non-empty queue becomes empty. */
+ queue->head = NULL;
+
+ return item;
+}
Index: include/grub/queue.h
===================================================================
--- include/grub/queue.h (revision 0)
+++ include/grub/queue.h (revision 0)
@@ -0,0 +1,52 @@
+/* queue.h - header for grub queue */
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_QUEUE_HEADER
+#define GRUB_QUEUE_HEADER 1
+
+#include <grub/symbol.h>
+#include <grub/types.h>
+#include <grub/list.h> /* GRUB_FIELD_MATCH */
+
+struct grub_queue_item
+{
+ struct grub_queue_item *next, *prev;
+};
+typedef struct grub_queue_item *grub_queue_item_t;
+
+struct grub_queue
+{
+ grub_queue_item_t head, tail;
+};
+typedef struct grub_queue *grub_queue_t;
+
+void EXPORT_FUNC(grub_queue_push) (grub_queue_t queue, grub_queue_item_t item);
+void * EXPORT_FUNC(grub_queue_pop) (grub_queue_t queue);
+
+extern void* grub_assert_fail (void);
+
+#define GRUB_AS_QUEUE_ITEM(ptr) \
+ (GRUB_FIELD_MATCH (ptr, grub_queue_item_t, next) && GRUB_FIELD_MATCH (ptr, grub_queue_item_t, prev) ? \
+ (grub_queue_item_t) ptr : grub_assert_fail ())
+
+#define GRUB_AS_QUEUE_ITEM_P(pptr) \
+ (GRUB_FIELD_MATCH (*pptr, grub_queue_item_t, next) && GRUB_FIELD_MATCH (*pptr, grub_queue_item_t, prev) ? \
+ (grub_queue_item_t *) (void *) pptr : grub_assert_fail ())
+
+#endif /* ! GRUB_QUEUE_HEADER */
Index: term/i386/pc/at_keyboard.c
===================================================================
--- term/i386/pc/at_keyboard.c (revision 2607)
+++ term/i386/pc/at_keyboard.c (working copy)
@@ -1,6 +1,6 @@
/*
* GRUB -- GRand Unified Bootloader
- * Copyright (C) 2007,2008 Free Software Foundation, Inc.
+ * Copyright (C) 2007,2008,2009 Free Software Foundation, Inc.
*
* GRUB is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -22,7 +22,52 @@
#include <grub/i386/io.h>
#include <grub/misc.h>
#include <grub/term.h>
+#include <grub/mm.h>
+#include <grub/queue.h>
+struct grub_at_keyboard_queue_item
+{
+ struct grub_at_keyboard_queue_item *next, *prev;
+ int key;
+};
+typedef struct grub_at_keyboard_queue_item *grub_at_keyboard_queue_item_t;
+
+static struct grub_queue grub_at_keyboard_queue;
+
+static void
+grub_at_keyboard_queue_push (int key)
+{
+ grub_at_keyboard_queue_item_t item;
+ item = grub_malloc (sizeof (*item));
+ item->key = key;
+ grub_queue_push (&grub_at_keyboard_queue, GRUB_AS_QUEUE_ITEM (item));
+}
+
+static int
+grub_at_keyboard_queue_pop (void)
+{
+ grub_at_keyboard_queue_item_t item;
+ int key;
+ item = grub_queue_pop (&grub_at_keyboard_queue);
+ if (item)
+ {
+ key = item->key;
+ grub_free (item);
+ }
+ else
+ key = -1;
+ return key;
+}
+
+static int
+grub_at_keyboard_queue_last (void)
+{
+ if (grub_at_keyboard_queue.tail)
+ return ((grub_at_keyboard_queue_item_t) grub_at_keyboard_queue.tail)->key;
+ else
+ return -1;
+}
+
static short at_keyboard_status = 0;
#define KEYBOARD_STATUS_SHIFT_L (1 << 0)
@@ -146,7 +191,7 @@ grub_keyboard_getkey (void)
/* If there is a character pending, return it; otherwise return -1. */
static int
-grub_at_keyboard_checkkey (void)
+grub_at_keyboard_getkey_noblock (void)
{
int code, key;
code = grub_keyboard_getkey ();
@@ -186,10 +231,24 @@ static int
key += 'a' - 'A';
}
}
- return (int) key;
+ return key;
}
static int
+grub_at_keyboard_checkkey (void)
+{
+ int key;
+ key = grub_at_keyboard_getkey_noblock ();
+
+ /* If there was a key, queue it. */
+ if (key != -1)
+ grub_at_keyboard_queue_push (key);
+
+ /* Return last key from queue. */
+ return grub_at_keyboard_queue_last ();
+}
+
+static int
grub_at_keyboard_getkey (void)
{
int key;
@@ -197,6 +256,10 @@ grub_at_keyboard_getkey (void)
{
key = grub_at_keyboard_checkkey ();
} while (key == -1);
+
+ /* Remove it from the queue. */
+ (void) grub_at_keyboard_queue_pop ();
+
return key;
}
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
