Re: [PATCH 6/8] libfshelp: fix the api of fshelp_set_active_translator

[Samuel Thibault]

Justus Winter, le Sun 12 Jan 2014 20:08:43 +0100, a écrit :
> To detect if an active translator goes away, we need to register for
> dead name notifications. Those notifications have to be sent to a port
> known to the ports library, as the ports library handles the dead name
> notifications. The most straight forward way is to use the port to the
> underlying node for that. To that end, a reference to the port_info
> struct is handed in and kept in the list of active translators.
> 
> This commit also moves the registration of dead name notifications to
> libfshelp.

Ack.

> * libfshelp/fshelp.h (fshelp_set_active_translator): Add port_info argument.
> * libfshelp/translator-list.c (struct translator): Add port_info pointer.
> (translator_ihash_cleanup): Dereference port_info object.
> (fshelp_set_active_translator): Register dead name notifications.
> * libdiskfs/file-set-trans.c (diskfs_S_file_set_translator): Update
>   accordingly.
> * libnetfs/file-set-translator.c (netfs_S_file_set_translator): Likewise.
> ---
>  libdiskfs/file-set-trans.c     | 18 +++---------------
>  libfshelp/fshelp.h             | 18 ++++++++++++------
>  libfshelp/translator-list.c    | 43 
> +++++++++++++++++++++++++++++++++++-------
>  libnetfs/file-set-translator.c | 19 ++++---------------
>  4 files changed, 55 insertions(+), 43 deletions(-)
> 
> diff --git a/libdiskfs/file-set-trans.c b/libdiskfs/file-set-trans.c
> index 8de2e64..58f6255 100644
> --- a/libdiskfs/file-set-trans.c
> +++ b/libdiskfs/file-set-trans.c
> @@ -1,5 +1,5 @@
>  /* libdiskfs implementation of fs.defs: file_set_translator
> -   Copyright (C) 1992,93,94,95,96,99,2001,02,13
> +   Copyright (C) 1992,93,94,95,96,99,2001,02,13,14
>       Free Software Foundation, Inc.
>  
>     This program is free software; you can redistribute it and/or
> @@ -208,20 +208,8 @@ diskfs_S_file_set_translator (struct protid *cred,
>  
>    pthread_mutex_unlock (&np->lock);
>  
> -  if (! error && cred->po->path)
> -    error = fshelp_set_active_translator (cred->po->path, active);
> -
> -  if (! error && active != MACH_PORT_NULL)
> -    {
> -      mach_port_t old;
> -      error = mach_port_request_notification (mach_task_self (), active,
> -                                           MACH_NOTIFY_DEAD_NAME, 0,
> -                                           cred->pi.port_right,
> -                                           MACH_MSG_TYPE_MAKE_SEND_ONCE,
> -                                           &old);
> -      if (old != MACH_PORT_NULL)
> -     mach_port_deallocate (mach_task_self (), old);
> -    }
> +  if (! error && cred->po->path && active_flags & FS_TRANS_SET)
> +    error = fshelp_set_active_translator (&cred->pi, cred->po->path, active);
>  
>    return error;
>  }
> diff --git a/libfshelp/fshelp.h b/libfshelp/fshelp.h
> index a7702ca..5d3a0ce 100644
> --- a/libfshelp/fshelp.h
> +++ b/libfshelp/fshelp.h
> @@ -1,5 +1,5 @@
>  /* FS helper library definitions
> -   Copyright (C) 1994,95,96,97,98,99,2000,01,02,13
> +   Copyright (C) 1994,95,96,97,98,99,2000,01,02,13,14
>       Free Software Foundation, Inc.
>  
>     This program is free software; you can redistribute it and/or
> @@ -34,14 +34,20 @@
>  
>  
>  /* Keeping track of active translators */
> -/* These routines keep a list of active translators.  They are
> -   self-contained and do not require multi threading or the ports
> -   library.  */
> +/* These routines keep a list of active translators.  They do not
> +   require multi threading but depend on the ports library.  */
> +
> +struct port_info;
>  
>  /* Record an active translator being bound to the given file name
> -   NAME.  ACTIVE is the control port of the translator.  */
> +   NAME.  ACTIVE is the control port of the translator.  PI references
> +   a receive port that is used to request dead name notifications,
> +   typically the port for the underlying node passed to the
> +   translator.  */
>  error_t
> -fshelp_set_active_translator (const char *name, mach_port_t active);
> +fshelp_set_active_translator (struct port_info *pi,
> +                              const char *name,
> +                              mach_port_t active);
>  
>  /* Remove the active translator specified by its control port ACTIVE.
>     If there is no active translator with the given control port, this
> diff --git a/libfshelp/translator-list.c b/libfshelp/translator-list.c
> index 87dcb21..3ece711 100644
> --- a/libfshelp/translator-list.c
> +++ b/libfshelp/translator-list.c
> @@ -1,6 +1,6 @@
>  /* A list of active translators.
>  
> -   Copyright (C) 2013 Free Software Foundation, Inc.
> +   Copyright (C) 2013,14 Free Software Foundation, Inc.
>  
>     Written by Justus Winter <4winter@informatik.uni-hamburg.de>
>  
> @@ -22,6 +22,7 @@
>  #include <argz.h>
>  #include <hurd/fsys.h>
>  #include <hurd/ihash.h>
> +#include <hurd/ports.h>
>  #include <mach.h>
>  #include <mach/notify.h>
>  #include <pthread.h>
> @@ -33,6 +34,7 @@
>  
>  struct translator
>  {
> +  struct port_info *pi;
>    char *name;
>    mach_port_t active;
>  };
> @@ -49,8 +51,10 @@ translator_ihash_cleanup (void *element, void *arg)
>  {
>    struct translator *translator = element;
>  
> -  /* No need to deallocate port, we only keep the name of the
> -     port, not a reference.  */
> +  if (translator->pi)
> +    ports_port_deref (translator->pi);
> +  /* No need to deallocate translator->active, we only keep the name of
> +     the port, not a reference.  */
>    free (translator->name);
>    free (translator);
>  }
> @@ -58,7 +62,9 @@ translator_ihash_cleanup (void *element, void *arg)
>  /* Record an active translator being bound to the given file name
>     NAME.  ACTIVE is the control port of the translator.  */
>  error_t
> -fshelp_set_active_translator (const char *name, mach_port_t active)
> +fshelp_set_active_translator (struct port_info *pi,
> +                           const char *name,
> +                           mach_port_t active)
>  {
>    error_t err = 0;
>    pthread_mutex_lock (&translator_ihash_lock);
> @@ -79,6 +85,7 @@ fshelp_set_active_translator (const char *name, mach_port_t 
> active)
>      return ENOMEM;
>  
>    t->active = MACH_PORT_NULL;
> +  t->pi = NULL;
>    t->name = strdup (name);
>    if (! t->name)
>      {
> @@ -93,9 +100,31 @@ fshelp_set_active_translator (const char *name, 
> mach_port_t active)
>  
>   update:
>    if (active)
> -    /* No need to increment the reference count, we only keep the
> -       name, not a reference.  */
> -    t->active = active;
> +    {
> +      if (t->pi != pi)
> +     {
> +       mach_port_t old;
> +       err = mach_port_request_notification (mach_task_self (), active,
> +                                             MACH_NOTIFY_DEAD_NAME, 0,
> +                                             pi->port_right,
> +                                             MACH_MSG_TYPE_MAKE_SEND_ONCE,
> +                                             &old);
> +       if (err)
> +         return err;
> +       if (old != MACH_PORT_NULL)
> +         mach_port_deallocate (mach_task_self (), old);
> +
> +       if (t->pi)
> +         ports_port_deref (t->pi);
> +
> +       ports_port_ref (pi);
> +       t->pi = pi;
> +     }
> +
> +      /* No need to increment the reference count, we only keep the
> +      name, not a reference.  */
> +      t->active = active;
> +    }
>    else
>      hurd_ihash_remove (&translator_ihash, (hurd_ihash_key_t) t);
>  
> diff --git a/libnetfs/file-set-translator.c b/libnetfs/file-set-translator.c
> index a9883a3..b46eb02 100644
> --- a/libnetfs/file-set-translator.c
> +++ b/libnetfs/file-set-translator.c
> @@ -1,5 +1,6 @@
>  /*
> -   Copyright (C) 1996, 1997, 1999, 2001, 2013 Free Software Foundation, Inc.
> +   Copyright (C) 1996, 1997, 1999, 2001, 2013, 2014
> +     Free Software Foundation, Inc.
>     Written by Michael I. Bushnell, p/BSG.
>  
>     This file is part of the GNU Hurd.
> @@ -175,20 +176,8 @@ netfs_S_file_set_translator (struct protid *user,
>       }
>      }
>  
> -  if (! err && user->po->path)
> -    err = fshelp_set_active_translator (user->po->path, active);
> -
> -  if (! err && active != MACH_PORT_NULL)
> -    {
> -      mach_port_t old;
> -      err = mach_port_request_notification (mach_task_self (), active,
> -                                         MACH_NOTIFY_DEAD_NAME, 0,
> -                                         user->pi.port_right,
> -                                         MACH_MSG_TYPE_MAKE_SEND_ONCE,
> -                                         &old);
> -      if (old != MACH_PORT_NULL)
> -     mach_port_deallocate (mach_task_self (), old);
> -    }
> +  if (! err && user->po->path && active_flags & FS_TRANS_SET)
> +    err = fshelp_set_active_translator (&user->pi, user->po->path, active);
>  
>   out:
>    pthread_mutex_unlock (&np->lock);
> -- 
> 1.8.5.2
> 

-- 
Samuel
<A> sauf que le firewall bloque tout sauf internet
 -+- ben ouais, il bloque ipx/spx ! -+-