Re: [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs

[Andrew Cooper <amc96@xxxxxxxx>]

On 16/01/2022 08:33, Juergen Gross wrote:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f960..c12341aa 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
>      return (intptr_t) h;
>  }
>  
> +static int xs_close_fd(struct file *file)
> +{
> +    struct xenbus_event *event, *next;
> +
> +    for (event = file->dev; event; event = next)
> +    {
> +        next = event->next;
> +        free(event);
> +    }
> +
> +    return 0;
> +}
> +
> +static bool xs_can_read(struct file *file)
> +{
> +    return file && file->dev;

Just 'return file->dev;' ?

> @@ -169,18 +202,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));

This is utterly mad.  In particular, close() looks to be very racy with
new watches arriving.

However, can the indentation at least be fixed here as the line is
changing.  That's a parameter to xenbus_watch_path_token(), not xs_bool().

> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678f..785389fb 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;
> +

Spurious hunk?

~Andrew