Re: [Xen-devel] [PATCH 18/25 v6] xen/arm: vpl011: Add a new console_cleanup function in xenconsole

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 17 Jul 2017, Bhupinder Thakur wrote:
> This patch introduces a new console_cleanup function. This function
> frees up the console resources.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> 
> Changes since v5:
> - Split this change in a separate patch.
> 
>  tools/console/daemon/io.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 4097673..d004687 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -769,22 +769,33 @@ static void remove_domain(struct domain *dom)
>       }
>  }
>  
> -static void cleanup_domain(struct domain *d)
> +static void console_cleanup(struct console *con)
>  {
> -     struct console *con = &d->console;
> -
> -     console_close_tty(con);
> -
>       if (con->log_fd != -1) {
>               close(con->log_fd);
>               con->log_fd = -1;
>       }
>  
> -     free(con->buffer.data);
> -     con->buffer.data = NULL;
> +     if (con->buffer.data)
> +     {

This is not the right code style for tools/console. Also, as Wei pointed
out, you can avoid the check and just call free.  Aside from these small
issues:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>



> +             free(con->buffer.data);
> +             con->buffer.data = NULL;
> +     }
> +
> +     if (con->xspath)
> +     {
> +             free(con->xspath);
> +             con->xspath = NULL;
> +     }
> +}
> +
> +static void cleanup_domain(struct domain *d)
> +{
> +     struct console *con = &d->console;
> +
> +     console_close_tty(con);
>  
> -     free(con->xspath);
> -     con->xspath = NULL;
> +     console_cleanup(con);
>  
>       remove_domain(d);
>  }
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel