Hi Grzegorz,
I've made a few coding-style comments inline.
Steven already answerd the most difficult questions, here is the
reminder:
--Apple-Mail-3--158434814
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream; x-unix-mode=0644;
name="minios.events.patch"
Content-Disposition: attachment;
filename=minios.events.patch
# HG changeset patch
# User gm281@xxxxxxxxxxxxxxxxxx
# Node ID ef01229d6344d7ff1c8970512fdb812445ec3430
# Parent c3d6610fc0f0510a09bd3d6c63a4e8a3f78312cc
Mini-OS: Events handling cleaned up. The interface extended to
provide void* pointer to handlers.
Signed-off-by: Steven Smith <sos22@xxxxxxxxx>
Signed-off-by: Grzegorz Milos <gm281@xxxxxxxxx>
diff -r c3d6610fc0f0 -r ef01229d6344 extras/mini-os/console/
xencons_ring.c
--- a/extras/mini-os/console/xencons_ring.c Wed Jul 5 10:06:14 2006
+++ b/extras/mini-os/console/xencons_ring.c Wed Jul 5 10:20:15 2006
@@ -53,7 +53,7 @@
-static void handle_input(int port, struct pt_regs *regs)
+static void handle_input(int port, struct pt_regs *regs, void *ign)
{
struct xencons_interface *intf = xencons_interface();
XENCONS_RING_IDX cons, prod;
@@ -83,7 +83,8 @@
if (!start_info.console_evtchn)
return 0;
- err = bind_evtchn(start_info.console_evtchn, handle_input);
+ err = bind_evtchn(start_info.console_evtchn, handle_input,
+ NULL);
Could you move the NULL ont the same line as bind_evtchn( ?
Would the line still be shorter then 80 characters? (that was the
reason to put NULL on the next line).
if (err <= 0) {
printk("XEN console request chn bind failed %i\n", err);
return err;
diff -r c3d6610fc0f0 -r ef01229d6344 extras/mini-os/events.c
--- a/extras/mini-os/events.c Wed Jul 5 10:06:14 2006
+++ b/extras/mini-os/events.c Wed Jul 5 10:20:15 2006
@@ -22,9 +22,18 @@
#include <events.h>
#include <lib.h>
+#define NR_EVS 1024
+
+/* this represents a event handler. Chaining or sharing is not
allowed */
+typedef struct _ev_action_t {
+ void (*handler)(int, struct pt_regs *, void *);
+ void *data;
+ u32 count;
+} ev_action_t;
+
Tabs and spaces are used inconsistently in the above structure
definition.
Ok - will change that.
static ev_action_t ev_actions[NR_EVS];
-void default_handler(int port, struct pt_regs *regs);
+void default_handler(int port, struct pt_regs *regs, void *data);
/*
@@ -35,42 +44,33 @@
ev_action_t *action;
if (port >= NR_EVS) {
printk("Port number too large: %d\n", port);
- goto out;
+ goto out;
}
action = &ev_actions[port];
action->count++;
- if (!action->handler)
- {
- printk("Spurious event on port %d\n", port);
- goto out;
- }
-
- if (action->status & EVS_DISABLED)
- {
- printk("Event on port %d disabled\n", port);
- goto out;
- }
-
/* call the handler */
- action->handler(port, regs);
-
+ action->handler(port, regs, action->data);
+
out:
clear_evtchn(port);
+
return 1;
}
-int bind_evtchn( u32 port, void (*handler)(int, struct pt_regs *) )
+int bind_evtchn( u32 port, void (*handler)(int, struct pt_regs *,
void *),
+ void *data )
{
if(ev_actions[port].handler != default_handler)
printk("WARN: Handler for port %d already registered,
replacing\n",
port);
+ ev_actions[port].data = data;
+ wmb();
ev_actions[port].handler = handler;
- ev_actions[port].status &= ~EVS_DISABLED;
-
+
/* Finally unmask the port */
unmask_evtchn(port);
@@ -82,13 +82,14 @@
if (ev_actions[port].handler == default_handler)
printk("WARN: No handler for port %d when unbinding\n", port);
ev_actions[port].handler = default_handler;
- ev_actions[port].status |= EVS_DISABLED;
+ wmb();
+ ev_actions[port].data = NULL;
}
-static void xenbus_evtchn_handler(int port, struct pt_regs *regs)
+static void xenbus_evtchn_handler(int port, struct pt_regs *regs,
void *ign)
{
wake_up(&xb_waitq);
}
@@ -174,7 +174,8 @@
create_thread("xenstore", xenbus_thread_func, NULL);
DEBUG("buf at %p.\n", xenstore_buf);
err = bind_evtchn(start_info.store_evtchn,
- xenbus_evtchn_handler);
+ xenbus_evtchn_handler,
+ NULL);
Again, could the NULL be appended to the previous line?
If the lines become too long to put more then one argument I
generally separate each argument into a new line. This seems to be
slightly more readable when you trying to find nth argument.
Cheers
Gregor
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home