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

Juergen Gross posted 12 patches 4 months, 1 week ago
[PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Juergen Gross 4 months, 1 week ago
Allocate the needed file type via alloc_file_type().

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

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 include/lib.h   |  3 +--
 lib/sys.c       | 18 --------------
 lib/xs.c        | 64 +++++++++++++++++++++++++++++++++++++------------
 xenbus/xenbus.c |  1 +
 4 files changed, 51 insertions(+), 35 deletions(-)

diff --git a/include/lib.h b/include/lib.h
index 283abb8..05c7de5 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -167,8 +167,7 @@ void sanity_check(void);
 #define FTYPE_BLK        9
 #define FTYPE_TPMFRONT  10
 #define FTYPE_TPM_TIS   11
-#define FTYPE_XENBUS    12
-#define FTYPE_N         13
+#define FTYPE_N         12
 #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 9540410..d213ae5 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -495,11 +495,6 @@ int close(int fd)
     switch (files[fd].type) {
         default:
             break;
-#ifdef CONFIG_XENBUS
-	case FTYPE_XENBUS:
-            xs_daemon_close((void*)(intptr_t) fd);
-            break;
-#endif
 #ifdef HAVE_LWIP
 	case FTYPE_SOCKET:
 	    res = lwip_close(files[fd].fd);
@@ -712,7 +707,6 @@ int closedir(DIR *dir)
 static const char *file_types[] = {
     [FTYPE_NONE]    = "none",
     [FTYPE_CONSOLE] = "console",
-    [FTYPE_XENBUS]  = "xenbus",
     [FTYPE_SOCKET]  = "socket",
     [FTYPE_TAP]     = "net",
     [FTYPE_BLK]     = "blk",
@@ -892,18 +886,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
                 n++;
 	    FD_CLR(i, exceptfds);
 	    break;
-#ifdef CONFIG_XENBUS
-	case FTYPE_XENBUS:
-	    if (FD_ISSET(i, readfds)) {
-                if (files[i].dev)
-		    n++;
-		else
-		    FD_CLR(i, readfds);
-	    }
-	    FD_CLR(i, writefds);
-	    FD_CLR(i, exceptfds);
-	    break;
-#endif
 	case FTYPE_TAP:
 	case FTYPE_BLK:
 	case FTYPE_KBD:
diff --git a/lib/xs.c b/lib/xs.c
index 4af0f96..ac830d2 100644
--- a/lib/xs.c
+++ b/lib/xs.c
@@ -18,23 +18,55 @@ static inline int _xs_fileno(struct xs_handle *h) {
     return (intptr_t) h;
 }
 
+static int xs_close_fd(int fd)
+{
+    struct xenbus_event *event, *next;
+    struct file *file = get_file_from_fd(fd);
+
+    for (event = file->dev; event; event = next)
+    {
+        next = event->next;
+        free(event);
+    }
+
+    return 0;
+}
+
+static bool xs_can_read(int fd)
+{
+    struct file *file = get_file_from_fd(fd);
+
+    return file && file->dev;
+}
+
+static struct file_ops xenbus_ops = {
+    .name = "xenbus",
+    .close = xs_close_fd,
+    .select_rd = xs_can_read,
+};
+
 struct xs_handle *xs_daemon_open()
 {
-    int fd = alloc_fd(FTYPE_XENBUS);
-    files[fd].dev = NULL;
-    printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
+    int fd;
+    struct file *file;
+    static unsigned int ftype_xenbus;
+
+    if ( !ftype_xenbus )
+        ftype_xenbus = alloc_file_type(&xenbus_ops);
+
+    fd = alloc_fd(ftype_xenbus);
+    file = get_file_from_fd(fd);
+    if ( !file )
+        return NULL;
+
+    file->dev = NULL;
+    printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
     return (void*)(intptr_t) fd;
 }
 
 void xs_daemon_close(struct xs_handle *h)
 {
-    int fd = _xs_fileno(h);
-    struct xenbus_event *event, *next;
-    for (event = files[fd].dev; event; event = next)
-    {
-        next = event->next;
-        free(event);
-    }
+    close(_xs_fileno(h));
 }
 
 int xs_fileno(struct xs_handle *h)
@@ -169,18 +201,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
 
 bool xs_watch(struct xs_handle *h, const char *path, const char *token)
 {
-    int fd = _xs_fileno(h);
+    struct file *file = get_file_from_fd(_xs_fileno(h));
+
     printk("xs_watch(%s, %s)\n", path, token);
     return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
-                   (xenbus_event_queue *)&files[fd].dev));
+                   (xenbus_event_queue *)&file->dev));
 }
 
 char **xs_read_watch(struct xs_handle *h, unsigned int *num)
 {
-    int fd = _xs_fileno(h);
     struct xenbus_event *event;
-    event = files[fd].dev;
-    files[fd].dev = event->next;
+    struct file *file = get_file_from_fd(_xs_fileno(h));
+
+    event = file->dev;
+    file->dev = event->next;
     printk("xs_read_watch() -> %s %s\n", event->path, event->token);
     *num = 2;
     return (char **) &event->path;
diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index b687678..785389f 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
 void init_xenbus(void)
 {
     int err;
+
     DEBUG("init_xenbus called.\n");
     create_thread("xenstore", xenbus_thread_func, NULL);
     DEBUG("buf at %p.\n", xenstore_buf);
-- 
2.26.2


Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Samuel Thibault 4 months, 1 week ago
Juergen Gross, le mar. 11 janv. 2022 16:12:06 +0100, a ecrit:
> Allocate the needed file type 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>

> ---
>  include/lib.h   |  3 +--
>  lib/sys.c       | 18 --------------
>  lib/xs.c        | 64 +++++++++++++++++++++++++++++++++++++------------
>  xenbus/xenbus.c |  1 +
>  4 files changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/include/lib.h b/include/lib.h
> index 283abb8..05c7de5 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -167,8 +167,7 @@ void sanity_check(void);
>  #define FTYPE_BLK        9
>  #define FTYPE_TPMFRONT  10
>  #define FTYPE_TPM_TIS   11
> -#define FTYPE_XENBUS    12
> -#define FTYPE_N         13
> +#define FTYPE_N         12
>  #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 9540410..d213ae5 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -495,11 +495,6 @@ int close(int fd)
>      switch (files[fd].type) {
>          default:
>              break;
> -#ifdef CONFIG_XENBUS
> -	case FTYPE_XENBUS:
> -            xs_daemon_close((void*)(intptr_t) fd);
> -            break;
> -#endif
>  #ifdef HAVE_LWIP
>  	case FTYPE_SOCKET:
>  	    res = lwip_close(files[fd].fd);
> @@ -712,7 +707,6 @@ int closedir(DIR *dir)
>  static const char *file_types[] = {
>      [FTYPE_NONE]    = "none",
>      [FTYPE_CONSOLE] = "console",
> -    [FTYPE_XENBUS]  = "xenbus",
>      [FTYPE_SOCKET]  = "socket",
>      [FTYPE_TAP]     = "net",
>      [FTYPE_BLK]     = "blk",
> @@ -892,18 +886,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
>                  n++;
>  	    FD_CLR(i, exceptfds);
>  	    break;
> -#ifdef CONFIG_XENBUS
> -	case FTYPE_XENBUS:
> -	    if (FD_ISSET(i, readfds)) {
> -                if (files[i].dev)
> -		    n++;
> -		else
> -		    FD_CLR(i, readfds);
> -	    }
> -	    FD_CLR(i, writefds);
> -	    FD_CLR(i, exceptfds);
> -	    break;
> -#endif
>  	case FTYPE_TAP:
>  	case FTYPE_BLK:
>  	case FTYPE_KBD:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f96..ac830d2 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,55 @@ static inline int _xs_fileno(struct xs_handle *h) {
>      return (intptr_t) h;
>  }
>  
> +static int xs_close_fd(int fd)
> +{
> +    struct xenbus_event *event, *next;
> +    struct file *file = get_file_from_fd(fd);
> +
> +    for (event = file->dev; event; event = next)
> +    {
> +        next = event->next;
> +        free(event);
> +    }
> +
> +    return 0;
> +}
> +
> +static bool xs_can_read(int fd)
> +{
> +    struct file *file = get_file_from_fd(fd);
> +
> +    return file && file->dev;
> +}
> +
> +static struct file_ops xenbus_ops = {
> +    .name = "xenbus",
> +    .close = xs_close_fd,
> +    .select_rd = xs_can_read,
> +};
> +
>  struct xs_handle *xs_daemon_open()
>  {
> -    int fd = alloc_fd(FTYPE_XENBUS);
> -    files[fd].dev = NULL;
> -    printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
> +    int fd;
> +    struct file *file;
> +    static unsigned int ftype_xenbus;
> +
> +    if ( !ftype_xenbus )
> +        ftype_xenbus = alloc_file_type(&xenbus_ops);
> +
> +    fd = alloc_fd(ftype_xenbus);
> +    file = get_file_from_fd(fd);
> +    if ( !file )
> +        return NULL;
> +
> +    file->dev = NULL;
> +    printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
>      return (void*)(intptr_t) fd;
>  }
>  
>  void xs_daemon_close(struct xs_handle *h)
>  {
> -    int fd = _xs_fileno(h);
> -    struct xenbus_event *event, *next;
> -    for (event = files[fd].dev; event; event = next)
> -    {
> -        next = event->next;
> -        free(event);
> -    }
> +    close(_xs_fileno(h));
>  }
>  
>  int xs_fileno(struct xs_handle *h)
> @@ -169,18 +201,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>  
>  bool xs_watch(struct xs_handle *h, const char *path, const char *token)
>  {
> -    int fd = _xs_fileno(h);
> +    struct file *file = get_file_from_fd(_xs_fileno(h));
> +
>      printk("xs_watch(%s, %s)\n", path, token);
>      return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> -                   (xenbus_event_queue *)&files[fd].dev));
> +                   (xenbus_event_queue *)&file->dev));
>  }
>  
>  char **xs_read_watch(struct xs_handle *h, unsigned int *num)
>  {
> -    int fd = _xs_fileno(h);
>      struct xenbus_event *event;
> -    event = files[fd].dev;
> -    files[fd].dev = event->next;
> +    struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> +    event = file->dev;
> +    file->dev = event->next;
>      printk("xs_read_watch() -> %s %s\n", event->path, event->token);
>      *num = 2;
>      return (char **) &event->path;
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678..785389f 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
>  void init_xenbus(void)
>  {
>      int err;
> +
>      DEBUG("init_xenbus called.\n");
>      create_thread("xenstore", xenbus_thread_func, NULL);
>      DEBUG("buf at %p.\n", xenstore_buf);
> -- 
> 2.26.2
> 

-- 
Samuel
<y> muhahaha...
<y> ya un train qui part de Perrache à 14h57
<y> qui passe à Part-Dieu à 15h10
<y> si je le prends à Perrache, je suis en zone bleue
<y> si je le prends à Part-Dieu, je suis en zone blanche
<y> donc je vais le prendre à Perrache *mais* à Part-Dieu ;-)
 -+- #ens-mim - vive la SNCF -+-

Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Samuel Thibault 4 months, 1 week ago
Samuel Thibault, le mar. 11 janv. 2022 21:06:15 +0100, a ecrit:
> Juergen Gross, le mar. 11 janv. 2022 16:12:06 +0100, a ecrit:
> > Allocate the needed file type 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>
> 
> > ---
> >  include/lib.h   |  3 +--
> >  lib/sys.c       | 18 --------------
> >  lib/xs.c        | 64 +++++++++++++++++++++++++++++++++++++------------
> >  xenbus/xenbus.c |  1 +
> >  4 files changed, 51 insertions(+), 35 deletions(-)
> > 
> > diff --git a/include/lib.h b/include/lib.h
> > index 283abb8..05c7de5 100644
> > --- a/include/lib.h
> > +++ b/include/lib.h
> > @@ -167,8 +167,7 @@ void sanity_check(void);
> >  #define FTYPE_BLK        9
> >  #define FTYPE_TPMFRONT  10
> >  #define FTYPE_TPM_TIS   11
> > -#define FTYPE_XENBUS    12
> > -#define FTYPE_N         13
> > +#define FTYPE_N         12
> >  #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 9540410..d213ae5 100644
> > --- a/lib/sys.c
> > +++ b/lib/sys.c
> > @@ -495,11 +495,6 @@ int close(int fd)
> >      switch (files[fd].type) {
> >          default:
> >              break;
> > -#ifdef CONFIG_XENBUS
> > -	case FTYPE_XENBUS:
> > -            xs_daemon_close((void*)(intptr_t) fd);
> > -            break;
> > -#endif
> >  #ifdef HAVE_LWIP
> >  	case FTYPE_SOCKET:
> >  	    res = lwip_close(files[fd].fd);
> > @@ -712,7 +707,6 @@ int closedir(DIR *dir)
> >  static const char *file_types[] = {
> >      [FTYPE_NONE]    = "none",
> >      [FTYPE_CONSOLE] = "console",
> > -    [FTYPE_XENBUS]  = "xenbus",
> >      [FTYPE_SOCKET]  = "socket",
> >      [FTYPE_TAP]     = "net",
> >      [FTYPE_BLK]     = "blk",
> > @@ -892,18 +886,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> >                  n++;
> >  	    FD_CLR(i, exceptfds);
> >  	    break;
> > -#ifdef CONFIG_XENBUS
> > -	case FTYPE_XENBUS:
> > -	    if (FD_ISSET(i, readfds)) {
> > -                if (files[i].dev)
> > -		    n++;
> > -		else
> > -		    FD_CLR(i, readfds);
> > -	    }
> > -	    FD_CLR(i, writefds);
> > -	    FD_CLR(i, exceptfds);
> > -	    break;
> > -#endif
> >  	case FTYPE_TAP:
> >  	case FTYPE_BLK:
> >  	case FTYPE_KBD:
> > diff --git a/lib/xs.c b/lib/xs.c
> > index 4af0f96..ac830d2 100644
> > --- a/lib/xs.c
> > +++ b/lib/xs.c
> > @@ -18,23 +18,55 @@ static inline int _xs_fileno(struct xs_handle *h) {
> >      return (intptr_t) h;
> >  }
> >  
> > +static int xs_close_fd(int fd)
> > +{
> > +    struct xenbus_event *event, *next;
> > +    struct file *file = get_file_from_fd(fd);

This...

> > +    for (event = file->dev; event; event = next)
> > +    {
> > +        next = event->next;
> > +        free(event);
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static bool xs_can_read(int fd)
> > +{
> > +    struct file *file = get_file_from_fd(fd);

... and that would be simplified by directly getting the file* from the
parameters.

> > +    return file && file->dev;
> > +}
> > +
> > +static struct file_ops xenbus_ops = {
> > +    .name = "xenbus",
> > +    .close = xs_close_fd,
> > +    .select_rd = xs_can_read,
> > +};
> > +
> >  struct xs_handle *xs_daemon_open()
> >  {
> > -    int fd = alloc_fd(FTYPE_XENBUS);
> > -    files[fd].dev = NULL;
> > -    printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
> > +    int fd;
> > +    struct file *file;
> > +    static unsigned int ftype_xenbus;
> > +
> > +    if ( !ftype_xenbus )
> > +        ftype_xenbus = alloc_file_type(&xenbus_ops);
> > +
> > +    fd = alloc_fd(ftype_xenbus);
> > +    file = get_file_from_fd(fd);
> > +    if ( !file )
> > +        return NULL;
> > +
> > +    file->dev = NULL;
> > +    printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
> >      return (void*)(intptr_t) fd;
> >  }
> >  
> >  void xs_daemon_close(struct xs_handle *h)
> >  {
> > -    int fd = _xs_fileno(h);
> > -    struct xenbus_event *event, *next;
> > -    for (event = files[fd].dev; event; event = next)
> > -    {
> > -        next = event->next;
> > -        free(event);
> > -    }
> > +    close(_xs_fileno(h));
> >  }
> >  
> >  int xs_fileno(struct xs_handle *h)
> > @@ -169,18 +201,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
> >  
> >  bool xs_watch(struct xs_handle *h, const char *path, const char *token)
> >  {
> > -    int fd = _xs_fileno(h);
> > +    struct file *file = get_file_from_fd(_xs_fileno(h));
> > +
> >      printk("xs_watch(%s, %s)\n", path, token);
> >      return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> > -                   (xenbus_event_queue *)&files[fd].dev));
> > +                   (xenbus_event_queue *)&file->dev));
> >  }
> >  
> >  char **xs_read_watch(struct xs_handle *h, unsigned int *num)
> >  {
> > -    int fd = _xs_fileno(h);
> >      struct xenbus_event *event;
> > -    event = files[fd].dev;
> > -    files[fd].dev = event->next;
> > +    struct file *file = get_file_from_fd(_xs_fileno(h));
> > +
> > +    event = file->dev;
> > +    file->dev = event->next;
> >      printk("xs_read_watch() -> %s %s\n", event->path, event->token);
> >      *num = 2;
> >      return (char **) &event->path;
> > diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> > index b687678..785389f 100644
> > --- a/xenbus/xenbus.c
> > +++ b/xenbus/xenbus.c
> > @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
> >  void init_xenbus(void)
> >  {
> >      int err;
> > +
> >      DEBUG("init_xenbus called.\n");
> >      create_thread("xenstore", xenbus_thread_func, NULL);
> >      DEBUG("buf at %p.\n", xenstore_buf);
> > -- 
> > 2.26.2
> > 
> 
> -- 
> Samuel
> <y> muhahaha...
> <y> ya un train qui part de Perrache à 14h57
> <y> qui passe à Part-Dieu à 15h10
> <y> si je le prends à Perrache, je suis en zone bleue
> <y> si je le prends à Part-Dieu, je suis en zone blanche
> <y> donc je vais le prendre à Perrache *mais* à Part-Dieu ;-)
>  -+- #ens-mim - vive la SNCF -+-

-- 
Samuel
<c> hiri, le cri ici, c des marrants
<c> j'ai un rep ".uglyhackdirectorywithoutacls" ds mon home
 -+- #ens-mim en stage -+-

Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Andrew Cooper 4 months, 1 week ago
On 11/01/2022 20:11, Samuel Thibault wrote:
> Samuel Thibault, le mar. 11 janv. 2022 21:06:15 +0100, a ecrit:
>> Juergen Gross, le mar. 11 janv. 2022 16:12:06 +0100, a ecrit:
>>> diff --git a/lib/xs.c b/lib/xs.c
>>> index 4af0f96..ac830d2 100644
>>> --- a/lib/xs.c
>>> +++ b/lib/xs.c
>>> @@ -18,23 +18,55 @@ static inline int _xs_fileno(struct xs_handle *h) {
>>>      return (intptr_t) h;
>>>  }
>>>  
>>> +static int xs_close_fd(int fd)
>>> +{
>>> +    struct xenbus_event *event, *next;
>>> +    struct file *file = get_file_from_fd(fd);
> This...
>
>>> +    for (event = file->dev; event; event = next)
>>> +    {
>>> +        next = event->next;
>>> +        free(event);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static bool xs_can_read(int fd)
>>> +{
>>> +    struct file *file = get_file_from_fd(fd);
> ... and that would be simplified by directly getting the file* from the
> parameters.

I actually make exactly the same suggestion for close() in
libxenevtchn.  The callback hooks don't need the same API as the libc
functions.

~Andrew

Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Andrew Cooper 4 months, 1 week ago
On 11/01/2022 15:12, Juergen Gross wrote:
>  void xs_daemon_close(struct xs_handle *h)
>  {
> -    int fd = _xs_fileno(h);
> -    struct xenbus_event *event, *next;
> -    for (event = files[fd].dev; event; event = next)
> -    {
> -        next = event->next;
> -        free(event);
> -    }
> +    close(_xs_fileno(h));
>  }

You've deleted the sole caller of xs_daemon_close() from the main
close() function.

That said, I'm very confused, because nothing in minios declares it. 
The declaration appears to come from xenstore.h, which is clearly
included unconditionally (when it ought not to be), but libxenstore also
defines the function too...

~Andrew

Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Juergen Gross 4 months, 1 week ago
On 11.01.22 21:21, Andrew Cooper wrote:
> On 11/01/2022 15:12, Juergen Gross wrote:
>>   void xs_daemon_close(struct xs_handle *h)
>>   {
>> -    int fd = _xs_fileno(h);
>> -    struct xenbus_event *event, *next;
>> -    for (event = files[fd].dev; event; event = next)
>> -    {
>> -        next = event->next;
>> -        free(event);
>> -    }
>> +    close(_xs_fileno(h));
>>   }
> 
> You've deleted the sole caller of xs_daemon_close() from the main
> close() function.
> 
> That said, I'm very confused, because nothing in minios declares it.
> The declaration appears to come from xenstore.h, which is clearly
> included unconditionally (when it ought not to be), but libxenstore also
> defines the function too...
I already thought of restructuring this mess.

lib/xs.c is the Mini-OS variant of libxenstore, and as such it shares
quite some code with libxenstore.

So the correct thing to do would be to split libxenstore into a common
part and a posix/Mini-OS part, drop lib/xs.c, and use the library in
Mini-OS instead.

But this should be done in a separate series.


Juergen
Re: [PATCH v2 03/12] mini-os: use alloc_file_type() and get_file_from_fd() in xs
Posted by Andrew Cooper 4 months, 1 week ago
On 12/01/2022 07:52, Juergen Gross wrote:
> On 11.01.22 21:21, Andrew Cooper wrote:
>> On 11/01/2022 15:12, Juergen Gross wrote:
>>>   void xs_daemon_close(struct xs_handle *h)
>>>   {
>>> -    int fd = _xs_fileno(h);
>>> -    struct xenbus_event *event, *next;
>>> -    for (event = files[fd].dev; event; event = next)
>>> -    {
>>> -        next = event->next;
>>> -        free(event);
>>> -    }
>>> +    close(_xs_fileno(h));
>>>   }
>>
>> You've deleted the sole caller of xs_daemon_close() from the main
>> close() function.
>>
>> That said, I'm very confused, because nothing in minios declares it.
>> The declaration appears to come from xenstore.h, which is clearly
>> included unconditionally (when it ought not to be), but libxenstore also
>> defines the function too...
> I already thought of restructuring this mess.
>
> lib/xs.c is the Mini-OS variant of libxenstore, and as such it shares
> quite some code with libxenstore.
>
> So the correct thing to do would be to split libxenstore into a common
> part and a posix/Mini-OS part, drop lib/xs.c, and use the library in
> Mini-OS instead.
>
> But this should be done in a separate series.

Sounds like a plan.

~Andrew