Re: [PATCH hurd 1/2] console: fix error and notification port handling

[Samuel Thibault]

Justus Winter, le Tue 20 Jan 2015 20:30:58 +0100, a écrit :
> This fixes a deallocation of an invalid port notably seen on system
> shutdown.

Ack, thanks!

> * console/display.c (free_modreqs): Handle errors of
> `mach_port_request_notification' and check whether the old
> notification port was valid.
> (do_mach_notify_msg_accepted): Likewise.
> (display_notice_filechange): Likewise.
> ---
>  console/display.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/console/display.c b/console/display.c
> index 09add5c..eb420fd 100644
> --- a/console/display.c
> +++ b/console/display.c
> @@ -298,16 +298,18 @@ nowait_file_changed (mach_port_t notify_port, natural_t 
> tickno,
>  static void
>  free_modreqs (struct modreq *mr)
>  {
> +  error_t err;
>    struct modreq *tmp;
>    for (; mr; mr = tmp)
>      {
>        mach_port_t old;
>        /* Cancel the dead-name notification.  */
> -      mach_port_request_notification (mach_task_self (), mr->port,
> -                                   MACH_NOTIFY_DEAD_NAME, 0,
> -                                   MACH_PORT_NULL,
> -                                   MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
> -      mach_port_deallocate (mach_task_self (), old);
> +      err = mach_port_request_notification (mach_task_self (), mr->port,
> +                                         MACH_NOTIFY_DEAD_NAME, 0,
> +                                         MACH_PORT_NULL,
> +                                         MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
> +      if (! err && MACH_PORT_VALID (old))
> +     mach_port_deallocate (mach_task_self(), old);
>  
>        /* Deallocate the user's port.  */
>        mach_port_deallocate (mach_task_self (), mr->port);
> @@ -438,16 +440,19 @@ do_mach_notify_msg_accepted (struct port_info *pi, 
> mach_port_t send)
>                                notify_port->pi.port_right);
>        if (err && err != MACH_SEND_WILL_NOTIFY)
>       {
> +       error_t e;
>         mach_port_t old;
>         *preq = req->next;
>         pthread_mutex_unlock (&display->lock);
>  
> -       /* Cancel the dead-name notification.  */
> -       mach_port_request_notification (mach_task_self (), req->port,
> -                                       MACH_NOTIFY_DEAD_NAME, 0,
> -                                       MACH_PORT_NULL,
> -                                       MACH_MSG_TYPE_MAKE_SEND_ONCE, &old);
> -       mach_port_deallocate (mach_task_self (), old);
> +       /* Cancel the dead-name notification.  */
> +       e = mach_port_request_notification (mach_task_self (), req->port,
> +                                           MACH_NOTIFY_DEAD_NAME, 0,
> +                                           MACH_PORT_NULL,
> +                                           MACH_MSG_TYPE_MAKE_SEND_ONCE,
> +                                           &old);
> +       if (! e && MACH_PORT_VALID (old))
> +         mach_port_deallocate (mach_task_self(), old);
>  
>         mach_port_deallocate (mach_task_self (), req->port);
>         free (req);
> @@ -564,13 +569,15 @@ display_notice_filechange (display_t display)
>           }
>         else
>           {
> +           error_t e;
>             mach_port_t old;
>  
>             /* Cancel the dead-name notification.  */
> -           mach_port_request_notification (mach_task_self (), req->port,
> -                                           MACH_NOTIFY_DEAD_NAME, 0,
> -                                           MACH_PORT_NULL, 0, &old);
> -           mach_port_deallocate (mach_task_self (), old);
> +           e = mach_port_request_notification (mach_task_self (), req->port,
> +                                               MACH_NOTIFY_DEAD_NAME, 0,
> +                                               MACH_PORT_NULL, 0, &old);
> +           if (! e && MACH_PORT_VALID (old))
> +             mach_port_deallocate (mach_task_self(), old);
>             mach_port_deallocate (mach_task_self (), req->port);
>             free (req);
>           }
> -- 
> 2.1.4
> 

-- 
Samuel
/* Halley */

        (Halley's comment.)