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>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
 fbfront.c     | 127 ++++++++++++++++++++++++++++++++++++++++++--------
 include/lib.h |   4 +-
 lib/sys.c     |  47 -------------------
 3 files changed, 108 insertions(+), 70 deletions(-)

diff --git a/fbfront.c b/fbfront.c
index 1e055fb0..360a9cf6 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,51 @@ close_kbdfront:
 }
 
 #ifdef HAVE_LIBC
+static int kbd_read(struct file *file, void *buf, size_t nbytes)
+{
+    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(struct file *file)
+{
+    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,
+};
+
+static unsigned int ftype_kbd;
+
+__attribute__((constructor))
+static void kbdfront_initialize(void)
+{
+    ftype_kbd = alloc_file_type(&kbd_ops);
+}
+
 int kbdfront_open(struct kbdfront_dev *dev)
 {
-    dev->fd = alloc_fd(FTYPE_KBD);
+    struct file *file;
+
+    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 +391,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 +418,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 +443,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 +746,51 @@ close_fbfront:
 }
 
 #ifdef HAVE_LIBC
+static int fbfront_read(struct file *file, void *buf, size_t nbytes)
+{
+    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(struct file *file)
+{
+    shutdown_fbfront(file->dev);
+
+    return 0;
+}
+
+static const struct file_ops fb_ops = {
+    .name = "fb",
+    .read = fbfront_read,
+    .close = fbfront_close_fd,
+    .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_fb;
+
+__attribute__((constructor))
+static void fbfront_initialize(void)
+{
+    ftype_fb = alloc_file_type(&fb_ops);
+}
+
 int fbfront_open(struct fbfront_dev *dev)
 {
-    dev->fd = alloc_fd(FTYPE_FB);
+    struct file *file;
+
+    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 b0d08807..b1beeed3 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -163,9 +163,7 @@ extern struct wait_queue_head event_queue;
 #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
 
 struct file {
diff --git a/lib/sys.c b/lib/sys.c
index 44fe848c..db26fd2d 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -308,30 +308,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;
@@ -471,16 +447,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:
@@ -666,8 +632,6 @@ static const char *const file_types[] = {
     [FTYPE_NONE]    = "none",
     [FTYPE_CONSOLE] = "console",
     [FTYPE_SOCKET]  = "socket",
-    [FTYPE_KBD]     = "kbd",
-    [FTYPE_FB]      = "fb",
 };
 
 static const char *get_type_name(unsigned int type)
@@ -850,17 +814,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

Juergen Gross, le dim. 16 janv. 2022 09:33:24 +0100, a ecrit:
> 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>

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

> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
>  fbfront.c     | 127 ++++++++++++++++++++++++++++++++++++++++++--------
>  include/lib.h |   4 +-
>  lib/sys.c     |  47 -------------------
>  3 files changed, 108 insertions(+), 70 deletions(-)
> 
> diff --git a/fbfront.c b/fbfront.c
> index 1e055fb0..360a9cf6 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,51 @@ close_kbdfront:
>  }
>  
>  #ifdef HAVE_LIBC
> +static int kbd_read(struct file *file, void *buf, size_t nbytes)
> +{
> +    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(struct file *file)
> +{
> +    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,
> +};
> +
> +static unsigned int ftype_kbd;
> +
> +__attribute__((constructor))
> +static void kbdfront_initialize(void)
> +{
> +    ftype_kbd = alloc_file_type(&kbd_ops);
> +}
> +
>  int kbdfront_open(struct kbdfront_dev *dev)
>  {
> -    dev->fd = alloc_fd(FTYPE_KBD);
> +    struct file *file;
> +
> +    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 +391,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 +418,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 +443,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 +746,51 @@ close_fbfront:
>  }
>  
>  #ifdef HAVE_LIBC
> +static int fbfront_read(struct file *file, void *buf, size_t nbytes)
> +{
> +    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(struct file *file)
> +{
> +    shutdown_fbfront(file->dev);
> +
> +    return 0;
> +}
> +
> +static const struct file_ops fb_ops = {
> +    .name = "fb",
> +    .read = fbfront_read,
> +    .close = fbfront_close_fd,
> +    .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_fb;
> +
> +__attribute__((constructor))
> +static void fbfront_initialize(void)
> +{
> +    ftype_fb = alloc_file_type(&fb_ops);
> +}
> +
>  int fbfront_open(struct fbfront_dev *dev)
>  {
> -    dev->fd = alloc_fd(FTYPE_FB);
> +    struct file *file;
> +
> +    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 b0d08807..b1beeed3 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -163,9 +163,7 @@ extern struct wait_queue_head event_queue;
>  #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
>  
>  struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index 44fe848c..db26fd2d 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -308,30 +308,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;
> @@ -471,16 +447,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:
> @@ -666,8 +632,6 @@ static const char *const file_types[] = {
>      [FTYPE_NONE]    = "none",
>      [FTYPE_CONSOLE] = "console",
>      [FTYPE_SOCKET]  = "socket",
> -    [FTYPE_KBD]     = "kbd",
> -    [FTYPE_FB]      = "fb",
>  };
>  
>  static const char *get_type_name(unsigned int type)
> @@ -850,17 +814,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
> 

-- 
Samuel
<B> l'alim je sais où elle est, elle est juste à côté de la dame qui dort
<g> B: clairement faut revoir les priorités dans la vie
<g> B: une dame ça se retrouve, un uptime...