Re: [Xen-devel] [PATCH] xen/console: Fix build when CONFIG_DEBUG_TRACE=y
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
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
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
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
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