diff mbox series

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

Message ID 1500296815-10243-19-git-send-email-bhupinder.thakur@linaro.org
State New
Headers show
Series SBSA UART emulation support in Xen | expand

Commit Message

Bhupinder Thakur July 17, 2017, 1:06 p.m. UTC 
This patch introduces a new console_cleanup function. This function
frees up the console resources.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

Changes since v5:
- Split this change in a separate patch.

 tools/console/daemon/io.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

Comments

Wei Liu July 18, 2017, 1:14 p.m. UTC | #1 
On Mon, Jul 17, 2017 at 06:36:48PM +0530, 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@linaro.org>
> ---
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> 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)
> +	{
> +		free(con->buffer.data);
> +		con->buffer.data = NULL;
> +	}
> +
> +	if (con->xspath)
> +	{
> +		free(con->xspath);
> +		con->xspath = NULL;
> +	}

The two if'es are not needed. free(3) can handle NULL just fine.
Stefano Stabellini July 18, 2017, 7:54 p.m. UTC | #2 
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@linaro.org>
> ---
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> 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@kernel.org>



> +		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
>
diff mbox series

Patch

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)
+	{
+		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);
 }