diff mbox series

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

Message ID 1500296815-10243-12-git-send-email-bhupinder.thakur@linaro.org
State Superseded
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_init function. This function
initializes the console structure.

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 | 64 +++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 25 deletions(-)

Comments

Wei Liu July 18, 2017, 11:40 a.m. UTC | #1 
On Mon, Jul 17, 2017 at 06:36:41PM +0530, Bhupinder Thakur wrote:
> This patch introduces a new console_init function. This function
> initializes the console structure.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>

I don't think this patch compiles, because ...

> ---
> 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 | 64 +++++++++++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index a2a3496..9e92097 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -655,13 +655,10 @@ static bool watch_domain(struct domain *dom, bool watch)
>  	return success;
>  }
>  
> -
> -static struct domain *create_domain(int domid)
> +static int console_init(struct console *con, struct domain *dom)
>  {
> -	struct domain *dom;
>  	char *s;
>  	struct timespec ts;
> -	struct console *con;
>  
>  	if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
>  		dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
> @@ -669,6 +666,41 @@ static struct domain *create_domain(int domid)
>  		return NULL;
>  	}
>  
> +	con->master_fd = -1;
> +	con->master_pollfd_idx = -1;
> +	con->slave_fd = -1;
> +	con->log_fd = -1;
> +	con->ring_ref = -1;
> +	con->local_port = -1;
> +	con->remote_port = -1;
> +	con->xce_pollfd_idx = -1;
> +	con->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD;
> +	con->d = dom;
> +	con->xspath = xs_get_domain_path(xs, dom->domid);
> +	s = realloc(con->xspath, strlen(con->xspath) +
> +		    strlen("/console") + 1);
> +	if (s)
> +	{
> +		con->xspath = s;
> +		strcat(con->xspath, "/console");
> +		err = 0;
> +	}
> +

err is not defined.

What I normally do to make sure patch split is correct is to

$ git rebase -i origin/staging -x 'make clean && make -j8 dist'

(something like that)
Bhupinder Thakur Aug. 7, 2017, 5:32 a.m. UTC | #2 
Hi Wei,



On 18 July 2017 at 17:10, Wei Liu <wei.liu2@citrix.com> wrote:
> On Mon, Jul 17, 2017 at 06:36:41PM +0530, Bhupinder Thakur wrote:
>> This patch introduces a new console_init function. This function
>> initializes the console structure.
>>
>> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
>
> I don't think this patch compiles, because ...
Thanks for pointing this out. I will fix this.
>
>> ---
>> 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 | 64 +++++++++++++++++++++++++++++------------------
>>  1 file changed, 39 insertions(+), 25 deletions(-)
>>
>> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
>> index a2a3496..9e92097 100644
>> --- a/tools/console/daemon/io.c
>> +++ b/tools/console/daemon/io.c
>> @@ -655,13 +655,10 @@ static bool watch_domain(struct domain *dom, bool watch)
>>       return success;
>>  }
>>
>> -
>> -static struct domain *create_domain(int domid)
>> +static int console_init(struct console *con, struct domain *dom)
>>  {
>> -     struct domain *dom;
>>       char *s;
>>       struct timespec ts;
>> -     struct console *con;
>>
>>       if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
>>               dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
>> @@ -669,6 +666,41 @@ static struct domain *create_domain(int domid)
>>               return NULL;
>>       }
>>
>> +     con->master_fd = -1;
>> +     con->master_pollfd_idx = -1;
>> +     con->slave_fd = -1;
>> +     con->log_fd = -1;
>> +     con->ring_ref = -1;
>> +     con->local_port = -1;
>> +     con->remote_port = -1;
>> +     con->xce_pollfd_idx = -1;
>> +     con->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD;
>> +     con->d = dom;
>> +     con->xspath = xs_get_domain_path(xs, dom->domid);
>> +     s = realloc(con->xspath, strlen(con->xspath) +
>> +                 strlen("/console") + 1);
>> +     if (s)
>> +     {
>> +             con->xspath = s;
>> +             strcat(con->xspath, "/console");
>> +             err = 0;
>> +     }
>> +
>
> err is not defined.
>
> What I normally do to make sure patch split is correct is to
>
> $ git rebase -i origin/staging -x 'make clean && make -j8 dist'

I will verify individual patch compilation.

Regards,
Bhupinder
diff mbox series

Patch

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index a2a3496..9e92097 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -655,13 +655,10 @@  static bool watch_domain(struct domain *dom, bool watch)
 	return success;
 }
 
-
-static struct domain *create_domain(int domid)
+static int console_init(struct console *con, struct domain *dom)
 {
-	struct domain *dom;
 	char *s;
 	struct timespec ts;
-	struct console *con;
 
 	if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
 		dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
@@ -669,6 +666,41 @@  static struct domain *create_domain(int domid)
 		return NULL;
 	}
 
+	con->master_fd = -1;
+	con->master_pollfd_idx = -1;
+	con->slave_fd = -1;
+	con->log_fd = -1;
+	con->ring_ref = -1;
+	con->local_port = -1;
+	con->remote_port = -1;
+	con->xce_pollfd_idx = -1;
+	con->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD;
+	con->d = dom;
+	con->xspath = xs_get_domain_path(xs, dom->domid);
+	s = realloc(con->xspath, strlen(con->xspath) +
+		    strlen("/console") + 1);
+	if (s)
+	{
+		con->xspath = s;
+		strcat(con->xspath, "/console");
+		err = 0;
+	}
+
+	return err;
+}
+
+static void console_free(struct console *con)
+{
+	if (con->xspath)
+		free(con->xspath);
+}
+
+static struct domain *create_domain(int domid)
+{
+	struct domain *dom;
+	char *s;
+	struct console *con;
+
 	dom = calloc(1, sizeof *dom);
 	if (dom == NULL) {
 		dolog(LOG_ERR, "Out of memory %s:%s():L%d",
@@ -677,28 +709,10 @@  static struct domain *create_domain(int domid)
 	}
 
 	dom->domid = domid;
-
 	con = &dom->console;
-	con->xspath = xs_get_domain_path(xs, dom->domid);
-	s = realloc(con->xspath, strlen(con->xspath) +
-		    strlen("/console") + 1);
-	if (s == NULL)
-		goto out;
-	con->xspath = s;
-	strcat(con->xspath, "/console");
 
-	con->master_fd = -1;
-	con->master_pollfd_idx = -1;
-	con->slave_fd = -1;
-	con->log_fd = -1;
-	con->xce_pollfd_idx = -1;
-	con->d = dom;
-
-	con->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD;
-
-	con->ring_ref = -1;
-	con->local_port = -1;
-	con->remote_port = -1;
+	if (console_init(con, dom))
+		goto out;
 
 	if (!watch_domain(dom, true))
 		goto out;
@@ -710,7 +724,7 @@  static struct domain *create_domain(int domid)
 
 	return dom;
  out:
-	free(con->xspath);
+	console_free(con);
 	free(dom);
 	return NULL;
 }