Re: [Xen-devel] [PATCH] tests/cpu-policy: fix format-overflow warning by null terminating strings

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jul 24, 2019 at 05:53:26PM -0700, christopher.w.clark@xxxxxxxxx wrote:
> From: Christopher Clark <christopher.w.clark@xxxxxxxxx>
> 
> gcc 9.1.0 reports:
> 
> | test-cpu-policy.c:64:18: error: '%.12s' directive argument is not a 
> nul-terminated string [-Werror=format-overflow=]
> |    64 |             fail("  Test '%.12s', expected vendor %u, got %u\n",
> |       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | test-cpu-policy.c:20:12: note: in definition of macro 'fail'
> |    20 |     printf(fmt, ##__VA_ARGS__);                 \
> |       |            ^~~
> | test-cpu-policy.c:64:27: note: format string is defined here
> |    64 |             fail("  Test '%.12s', expected vendor %u, got %u\n",
> |       |                           ^~~~~
> | test-cpu-policy.c:44:7: note: referenced argument declared here
> |    44 |     } tests[] = {
> |       |       ^~~~~
> 
> so increase the string array size by one character for the null string
> terminator and add another single char to the struct within the same
> union to balance it.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@xxxxxxxxxxxxxx>

Thanks! Just two nits below.

> ---
>  tools/tests/cpu-policy/test-cpu-policy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/tests/cpu-policy/test-cpu-policy.c 
> b/tools/tests/cpu-policy/test-cpu-policy.c
> index ca3b8dd45f..c91408a93a 100644
> --- a/tools/tests/cpu-policy/test-cpu-policy.c
> +++ b/tools/tests/cpu-policy/test-cpu-policy.c
> @@ -35,9 +35,10 @@ static void test_vendor_identification(void)
>  {
>      static const struct test {
>          union {
> -            char ident[12];
> +            char ident[13];
>              struct {
>                  uint32_t b, d, c;
> +                char null_terminator;

Do you really need this field here, AFAICT it's unused. The compiler
will add the padding here automatically to match the size of the other
field of the union.

Also, since ident is null terminated now you can remove the .12 from
the format string.

Thanks, Roger.

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