[PATCH v2 08/12] mini-os: use alloc_file_type() and get_file_from_fd() in fbfront

Juergen Gross posted 12 patches 4 months, 1 week ago
[PATCH v2 08/12] mini-os: use alloc_file_type() and get_file_from_fd() in fbfront
Posted by Juergen Gross 4 months, 1 week ago
Allocate file types dynamically via alloc_file_type().

Instead of directly accessing the files[] array use get_file_from_fd().

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 fbfront.c     | 125 ++++++++++++++++++++++++++++++++++++++++++--------
 include/lib.h |   4 +-
 lib/sys.c     |  47 -------------------
 3 files changed, 106 insertions(+), 70 deletions(-)

diff --git a/fbfront.c b/fbfront.c
index 1e055fb..9c96963 100644
--- a/fbfront.c
+++ b/fbfront.c
@@ -14,6 +14,9 @@
 #include <mini-os/xmalloc.h>
 #include <mini-os/fbfront.h>
 #include <mini-os/lib.h>
+#ifdef HAVE_LIBC
+#include <errno.h>
+#endif
 
 DECLARE_WAIT_QUEUE_HEAD(kbdfront_queue);
 
@@ -42,10 +45,10 @@ void kbdfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
 #ifdef HAVE_LIBC
     struct kbdfront_dev *dev = data;
-    int fd = dev->fd;
+    struct file *file = get_file_from_fd(dev->fd);
 
-    if (fd != -1)
-        files[fd].read = true;
+    if ( file )
+        file->read = true;
 #endif
     wake_up(&kbdfront_queue);
 }
@@ -204,10 +207,12 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
     struct xenkbd_page *page = dev->page;
     uint32_t prod, cons;
     int i;
-
 #ifdef HAVE_LIBC
-    if (dev->fd != -1) {
-        files[dev->fd].read = false;
+    struct file *file = get_file_from_fd(dev->fd);
+
+    if ( file )
+    {
+        file->read = false;
         mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
     }
 #endif
@@ -227,9 +232,9 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
     notify_remote_via_evtchn(dev->evtchn);
 
 #ifdef HAVE_LIBC
-    if (cons != prod && dev->fd != -1)
+    if ( cons != prod && file )
         /* still some events to read */
-        files[dev->fd].read = true;
+        file->read = true;
 #endif
 
     return i;
@@ -298,11 +303,50 @@ close_kbdfront:
 }
 
 #ifdef HAVE_LIBC
+static int kbd_read(int fd, void *buf, size_t nbytes)
+{
+    struct file *file = get_file_from_fd(fd);
+    int ret, n;
+
+    n = nbytes / sizeof(union xenkbd_in_event);
+    ret = kbdfront_receive(file->dev, buf, n);
+    if ( ret <= 0 )
+    {
+        errno = EAGAIN;
+        return -1;
+    }
+
+    return ret * sizeof(union xenkbd_in_event);
+}
+
+static int kbd_close_fd(int fd)
+{
+    struct file *file = get_file_from_fd(fd);
+
+    shutdown_kbdfront(file->dev);
+
+    return 0;
+}
+
+static struct file_ops kbd_ops = {
+    .name = "kbd",
+    .read = kbd_read,
+    .close = kbd_close_fd,
+    .select_rd = select_read_flag,
+};
+
 int kbdfront_open(struct kbdfront_dev *dev)
 {
-    dev->fd = alloc_fd(FTYPE_KBD);
+    struct file *file;
+    static unsigned int ftype_kbd;
+
+    if ( !ftype_kbd )
+        ftype_kbd = alloc_file_type(&kbd_ops);
+
+    dev->fd = alloc_fd(ftype_kbd);
     printk("kbd_open(%s) -> %d\n", dev->nodename, dev->fd);
-    files[dev->fd].dev = dev;
+    file = get_file_from_fd(dev->fd);
+    file->dev = dev;
     return dev->fd;
 }
 #endif
@@ -346,10 +390,10 @@ void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
 #ifdef HAVE_LIBC
     struct fbfront_dev *dev = data;
-    int fd = dev->fd;
+    struct file *file = get_file_from_fd(dev->fd);
 
-    if (fd != -1)
-        files[fd].read = true;
+    if ( file )
+        file->read = true;
 #endif
     wake_up(&fbfront_queue);
 }
@@ -373,10 +417,12 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
     struct xenfb_page *page = dev->page;
     uint32_t prod, cons;
     int i;
-
 #ifdef HAVE_LIBC
-    if (dev->fd != -1) {
-        files[dev->fd].read = false;
+    struct file *file = get_file_from_fd(dev->fd);
+
+    if ( file )
+    {
+        file->read = false;
         mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
     }
 #endif
@@ -396,9 +442,9 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
     notify_remote_via_evtchn(dev->evtchn);
 
 #ifdef HAVE_LIBC
-    if (cons != prod && dev->fd != -1)
+    if ( cons != prod && file )
         /* still some events to read */
-        files[dev->fd].read = true;
+        file->read = true;
 #endif
 
     return i;
@@ -699,11 +745,50 @@ close_fbfront:
 }
 
 #ifdef HAVE_LIBC
+static int fbfront_read(int fd, void *buf, size_t nbytes)
+{
+    struct file *file = get_file_from_fd(fd);
+    int ret, n;
+
+    n = nbytes / sizeof(union xenfb_in_event);
+    ret = fbfront_receive(file->dev, buf, n);
+    if ( ret <= 0 )
+    {
+        errno = EAGAIN;
+        return -1;
+    }
+
+    return ret * sizeof(union xenfb_in_event);
+}
+
+static int fbfront_close_fd(int fd)
+{
+    struct file *file = get_file_from_fd(fd);
+
+    shutdown_fbfront(file->dev);
+
+    return 0;
+}
+
+static struct file_ops fb_ops = {
+    .name = ".fb",
+    .read = fbfront_read,
+    .close = fbfront_close_fd,
+    .select_rd = select_read_flag,
+};
+
 int fbfront_open(struct fbfront_dev *dev)
 {
-    dev->fd = alloc_fd(FTYPE_FB);
+    struct file *file;
+    static unsigned int ftype_fb;
+
+    if ( !ftype_fb )
+        ftype_fb = alloc_file_type(&fb_ops);
+
+    dev->fd = alloc_fd(ftype_fb);
     printk("fb_open(%s) -> %d\n", dev->nodename, dev->fd);
-    files[dev->fd].dev = dev;
+    file = get_file_from_fd(dev->fd);
+    file->dev = dev;
     return dev->fd;
 }
 #endif
diff --git a/include/lib.h b/include/lib.h
index aa8036e..653a16e 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -161,9 +161,7 @@ void sanity_check(void);
 #define FTYPE_SOCKET     3
 #define FTYPE_MEM        4
 #define FTYPE_SAVEFILE   5
-#define FTYPE_FB         6
-#define FTYPE_KBD        7
-#define FTYPE_N          8
+#define FTYPE_N          6
 #define FTYPE_SPARE     16
 
 typedef int file_read_func(int fd, void *buf, size_t nbytes);
diff --git a/lib/sys.c b/lib/sys.c
index 7f2e11f..4fb844f 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -302,30 +302,6 @@ int read(int fd, void *buf, size_t nbytes)
 #ifdef HAVE_LWIP
 	case FTYPE_SOCKET:
 	    return lwip_read(files[fd].fd, buf, nbytes);
-#endif
-#ifdef CONFIG_KBDFRONT
-        case FTYPE_KBD: {
-            int ret, n;
-            n = nbytes / sizeof(union xenkbd_in_event);
-            ret = kbdfront_receive(files[fd].dev, buf, n);
-	    if (ret <= 0) {
-		errno = EAGAIN;
-		return -1;
-	    }
-	    return ret * sizeof(union xenkbd_in_event);
-        }
-#endif
-#ifdef CONFIG_FBFRONT
-        case FTYPE_FB: {
-            int ret, n;
-            n = nbytes / sizeof(union xenfb_in_event);
-            ret = fbfront_receive(files[fd].dev, buf, n);
-	    if (ret <= 0) {
-		errno = EAGAIN;
-		return -1;
-	    }
-	    return ret * sizeof(union xenfb_in_event);
-        }
 #endif
 	default:
 	    break;
@@ -443,16 +419,6 @@ int close(int fd)
 	    res = lwip_close(files[fd].fd);
             break;
 #endif
-#ifdef CONFIG_KBDFRONT
-	case FTYPE_KBD:
-            shutdown_kbdfront(files[fd].dev);
-            break;
-#endif
-#ifdef CONFIG_FBFRONT
-	case FTYPE_FB:
-            shutdown_fbfront(files[fd].dev);
-            break;
-#endif
 #ifdef CONFIG_CONSFRONT
         case FTYPE_SAVEFILE:
         case FTYPE_CONSOLE:
@@ -619,8 +585,6 @@ static const char *file_types[] = {
     [FTYPE_NONE]    = "none",
     [FTYPE_CONSOLE] = "console",
     [FTYPE_SOCKET]  = "socket",
-    [FTYPE_KBD]     = "kbd",
-    [FTYPE_FB]      = "fb",
 };
 
 static char *get_type_name(unsigned int type)
@@ -795,17 +759,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
                 n++;
 	    FD_CLR(i, exceptfds);
 	    break;
-	case FTYPE_KBD:
-	case FTYPE_FB:
-	    if (FD_ISSET(i, readfds)) {
-		if (files[i].read)
-		    n++;
-		else
-		    FD_CLR(i, readfds);
-	    }
-	    FD_CLR(i, writefds);
-	    FD_CLR(i, exceptfds);
-	    break;
 #ifdef HAVE_LWIP
 	case FTYPE_SOCKET:
 	    if (FD_ISSET(i, readfds)) {
-- 
2.26.2


Re: [PATCH v2 08/12] mini-os: use alloc_file_type() and get_file_from_fd() in fbfront
Posted by Samuel Thibault 4 months, 1 week ago
Juergen Gross, le mar. 11 janv. 2022 16:12:11 +0100, a ecrit:
> +static struct file_ops fb_ops = {
> +    .name = ".fb",

Why a dot?


Apart from that, and modulo the int fd / file * thing,

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>


Re: [PATCH v2 08/12] mini-os: use alloc_file_type() and get_file_from_fd() in fbfront
Posted by Juergen Gross 4 months, 1 week ago
On 11.01.22 21:26, Samuel Thibault wrote:
> Juergen Gross, le mar. 11 janv. 2022 16:12:11 +0100, a ecrit:
>> +static struct file_ops fb_ops = {
>> +    .name = ".fb",
> 
> Why a dot?

I had spare one lying around... ;-)

Thanks for noticing!


Juergen