Re: [Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y

2019-08-21 Thread Julien Grall

Hi,

On 20/08/2019 14:12, Andrew Cooper wrote:

On 19/08/2019 19:37, Julien Grall wrote:

Hi Andrew,

On 8/19/19 7:04 PM, Andrew Cooper wrote:

On 19/08/2019 19:01, Julien Grall wrote:

Commit b5e6e1ee8da "xen/console: Don't treat NUL character as the end
of the buffer" extended sercon_puts to take the number of character
to print in argument.

Sadly, a couple of couple of the callers in debugtrace_dump_worker()
were not converted. This result to a build failure when enabling
CONFIG_DEBUG_TRACE.

Spotted by Travis using randconfig
Signed-off-by: Julien Grall 
---
   xen/drivers/char/console.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2c14c2ca73..924d4971ca 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1185,11 +1185,12 @@ static void debugtrace_dump_worker(void)
     /* Print oldest portion of the ring. */
   ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
-    sercon_puts(_buf[debugtrace_prd]);
+    sercon_puts(_buf[debugtrace_prd],
+    strlen(_buf[debugtrace_prd]));


Isn't this just debugtrace_bytes - debugtrace_prd - 1 ?


I tried and it resulted to print a lot of @^ on the console. This is
because the ring may not be full.

So the portion between debugtrace_prd and debugtrace_bytes will be
full of zero.

Looking at the code again, I think this portion and either be full of
zero character or full of non-zero character. In other word, a mix
would not be possible. So how about:

if ( debugtrace_buf[debugtrace_prd] != '\0' )
   sercon_puts(_buf[debugtrace_prd],
   debugtrace_bytes - debugtrace_prd - 1);


LGTM.  Acked-by: Andrew Cooper 


Thank you! I will update the patch and commit it.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y

2019-08-20 Thread Andrew Cooper
On 19/08/2019 19:37, Julien Grall wrote:
> Hi Andrew,
>
> On 8/19/19 7:04 PM, Andrew Cooper wrote:
>> On 19/08/2019 19:01, Julien Grall wrote:
>>> Commit b5e6e1ee8da "xen/console: Don't treat NUL character as the end
>>> of the buffer" extended sercon_puts to take the number of character
>>> to print in argument.
>>>
>>> Sadly, a couple of couple of the callers in debugtrace_dump_worker()
>>> were not converted. This result to a build failure when enabling
>>> CONFIG_DEBUG_TRACE.
>>>
>>> Spotted by Travis using randconfig
>>> Signed-off-by: Julien Grall 
>>> ---
>>>   xen/drivers/char/console.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
>>> index 2c14c2ca73..924d4971ca 100644
>>> --- a/xen/drivers/char/console.c
>>> +++ b/xen/drivers/char/console.c
>>> @@ -1185,11 +1185,12 @@ static void debugtrace_dump_worker(void)
>>>     /* Print oldest portion of the ring. */
>>>   ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
>>> -    sercon_puts(_buf[debugtrace_prd]);
>>> +    sercon_puts(_buf[debugtrace_prd],
>>> +    strlen(_buf[debugtrace_prd]));
>>
>> Isn't this just debugtrace_bytes - debugtrace_prd - 1 ?
>
> I tried and it resulted to print a lot of @^ on the console. This is
> because the ring may not be full.
>
> So the portion between debugtrace_prd and debugtrace_bytes will be
> full of zero.
>
> Looking at the code again, I think this portion and either be full of
> zero character or full of non-zero character. In other word, a mix
> would not be possible. So how about:
>
> if ( debugtrace_buf[debugtrace_prd] != '\0' )
>   sercon_puts(_buf[debugtrace_prd],
>   debugtrace_bytes - debugtrace_prd - 1);

LGTM.  Acked-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y

2019-08-19 Thread Julien Grall

Hi Andrew,

On 8/19/19 7:04 PM, Andrew Cooper wrote:

On 19/08/2019 19:01, Julien Grall wrote:

Commit b5e6e1ee8da "xen/console: Don't treat NUL character as the end
of the buffer" extended sercon_puts to take the number of character
to print in argument.

Sadly, a couple of couple of the callers in debugtrace_dump_worker()
were not converted. This result to a build failure when enabling
CONFIG_DEBUG_TRACE.

Spotted by Travis using randconfig
Signed-off-by: Julien Grall 
---
  xen/drivers/char/console.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2c14c2ca73..924d4971ca 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1185,11 +1185,12 @@ static void debugtrace_dump_worker(void)
  
  /* Print oldest portion of the ring. */

  ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
-sercon_puts(_buf[debugtrace_prd]);
+sercon_puts(_buf[debugtrace_prd],
+strlen(_buf[debugtrace_prd]));


Isn't this just debugtrace_bytes - debugtrace_prd - 1 ?


I tried and it resulted to print a lot of @^ on the console. This is 
because the ring may not be full.


So the portion between debugtrace_prd and debugtrace_bytes will be full 
of zero.


Looking at the code again, I think this portion and either be full of 
zero character or full of non-zero character. In other word, a mix would 
not be possible. So how about:


if ( debugtrace_buf[debugtrace_prd] != '\0' )
  sercon_puts(_buf[debugtrace_prd],
  debugtrace_bytes - debugtrace_prd - 1);


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y

2019-08-19 Thread Andrew Cooper
On 19/08/2019 19:01, Julien Grall wrote:
> Commit b5e6e1ee8da "xen/console: Don't treat NUL character as the end
> of the buffer" extended sercon_puts to take the number of character
> to print in argument.
>
> Sadly, a couple of couple of the callers in debugtrace_dump_worker()
> were not converted. This result to a build failure when enabling
> CONFIG_DEBUG_TRACE.
>
> Spotted by Travis using randconfig
> Signed-off-by: Julien Grall 
> ---
>  xen/drivers/char/console.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 2c14c2ca73..924d4971ca 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -1185,11 +1185,12 @@ static void debugtrace_dump_worker(void)
>  
>  /* Print oldest portion of the ring. */
>  ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
> -sercon_puts(_buf[debugtrace_prd]);
> +sercon_puts(_buf[debugtrace_prd],
> +strlen(_buf[debugtrace_prd]));

Isn't this just debugtrace_bytes - debugtrace_prd - 1 ?

~Andrew

>  
>  /* Print youngest portion of the ring. */
>  debugtrace_buf[debugtrace_prd] = '\0';
> -sercon_puts(_buf[0]);
> +sercon_puts(_buf[0], debugtrace_prd);
>  
>  memset(debugtrace_buf, '\0', debugtrace_bytes);
>  


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y

2019-08-19 Thread Julien Grall
Commit b5e6e1ee8da "xen/console: Don't treat NUL character as the end
of the buffer" extended sercon_puts to take the number of character
to print in argument.

Sadly, a couple of couple of the callers in debugtrace_dump_worker()
were not converted. This result to a build failure when enabling
CONFIG_DEBUG_TRACE.

Spotted by Travis using randconfig
Signed-off-by: Julien Grall 
---
 xen/drivers/char/console.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 2c14c2ca73..924d4971ca 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1185,11 +1185,12 @@ static void debugtrace_dump_worker(void)
 
 /* Print oldest portion of the ring. */
 ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
-sercon_puts(_buf[debugtrace_prd]);
+sercon_puts(_buf[debugtrace_prd],
+strlen(_buf[debugtrace_prd]));
 
 /* Print youngest portion of the ring. */
 debugtrace_buf[debugtrace_prd] = '\0';
-sercon_puts(_buf[0]);
+sercon_puts(_buf[0], debugtrace_prd);
 
 memset(debugtrace_buf, '\0', debugtrace_bytes);
 
-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel