[kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[Thomas Huth]

This way, the code can be compiled with "-Wwrite-strings", too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Not sure whether we want to enable "-Wwrite-strings" globally (since
 strings in kvm-unit-test are theoretically writable if the test is
 running without MMU), so I only fixed the code here, without adding
 it to the Makefile. But if we agree that it is a good idea I can
 respin the patch and add it to the Makefile, too.

 lib/report.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/report.c b/lib/report.c
index b002d21..c0a701f 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -81,9 +81,9 @@ void report_prefix_pop(void)
 static void va_report(const char *msg_fmt,
 		bool pass, bool xfail, bool skip, va_list va)
 {
-	char *prefix = skip ? "SKIP"
-	                    : xfail ? (pass ? "XPASS" : "XFAIL")
-	                            : (pass ? "PASS"  : "FAIL");
+	const char *prefix = skip ? "SKIP"
+				  : xfail ? (pass ? "XPASS" : "XFAIL")
+				  : (pass ? "PASS"  : "FAIL");
 
 	spin_lock(&lock);
 
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[David Hildenbrand]

On 27.06.2017 07:09, Thomas Huth wrote:
> This way, the code can be compiled with "-Wwrite-strings", too.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  Not sure whether we want to enable "-Wwrite-strings" globally (since
>  strings in kvm-unit-test are theoretically writable if the test is
>  running without MMU), so I only fixed the code here, without adding
>  it to the Makefile. But if we agree that it is a good idea I can
>  respin the patch and add it to the Makefile, too.

Make sense to me. I'd vote for adding it as long as all targets can be
compiled without any problems.

> 
>  lib/report.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/report.c b/lib/report.c
> index b002d21..c0a701f 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -81,9 +81,9 @@ void report_prefix_pop(void)
>  static void va_report(const char *msg_fmt,
>  		bool pass, bool xfail, bool skip, va_list va)
>  {
> -	char *prefix = skip ? "SKIP"
> -	                    : xfail ? (pass ? "XPASS" : "XFAIL")
> -	                            : (pass ? "PASS"  : "FAIL");
> +	const char *prefix = skip ? "SKIP"
> +				  : xfail ? (pass ? "XPASS" : "XFAIL")
> +				  : (pass ? "PASS"  : "FAIL");

Hmm, think I liked the old indentation better. (":" directly below the
matching "?")

>  
>  	spin_lock(&lock);
>  
> 


-- 

Thanks,

David

Re: [kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[Thomas Huth]

On 27.06.2017 10:37, David Hildenbrand wrote:
> On 27.06.2017 07:09, Thomas Huth wrote:
>> This way, the code can be compiled with "-Wwrite-strings", too.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  Not sure whether we want to enable "-Wwrite-strings" globally (since
>>  strings in kvm-unit-test are theoretically writable if the test is
>>  running without MMU), so I only fixed the code here, without adding
>>  it to the Makefile. But if we agree that it is a good idea I can
>>  respin the patch and add it to the Makefile, too.
> 
> Make sense to me. I'd vote for adding it as long as all targets can be
> compiled without any problems.
> 
>>
>>  lib/report.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/report.c b/lib/report.c
>> index b002d21..c0a701f 100644
>> --- a/lib/report.c
>> +++ b/lib/report.c
>> @@ -81,9 +81,9 @@ void report_prefix_pop(void)
>>  static void va_report(const char *msg_fmt,
>>  		bool pass, bool xfail, bool skip, va_list va)
>>  {
>> -	char *prefix = skip ? "SKIP"
>> -	                    : xfail ? (pass ? "XPASS" : "XFAIL")
>> -	                            : (pass ? "PASS"  : "FAIL");
>> +	const char *prefix = skip ? "SKIP"
>> +				  : xfail ? (pass ? "XPASS" : "XFAIL")
>> +				  : (pass ? "PASS"  : "FAIL");
> 
> Hmm, think I liked the old indentation better. (":" directly below the
> matching "?")

Oh, right, thanks! That apparently happens when sending patches too
early in the morning ;-)
I'll wait for some more comments about whether to include
"-Wwrite-strings" or not, then I'll respin this patch.

 Thomas

Re: [kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[Andrew Jones]

On Tue, Jun 27, 2017 at 10:39:39AM +0200, Thomas Huth wrote:
> On 27.06.2017 10:37, David Hildenbrand wrote:
> > On 27.06.2017 07:09, Thomas Huth wrote:
> >> This way, the code can be compiled with "-Wwrite-strings", too.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  Not sure whether we want to enable "-Wwrite-strings" globally (since
> >>  strings in kvm-unit-test are theoretically writable if the test is
> >>  running without MMU), so I only fixed the code here, without adding
> >>  it to the Makefile. But if we agree that it is a good idea I can
> >>  respin the patch and add it to the Makefile, too.
> > 
> > Make sense to me. I'd vote for adding it as long as all targets can be
> > compiled without any problems.
> > 
> >>
> >>  lib/report.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/lib/report.c b/lib/report.c
> >> index b002d21..c0a701f 100644
> >> --- a/lib/report.c
> >> +++ b/lib/report.c
> >> @@ -81,9 +81,9 @@ void report_prefix_pop(void)
> >>  static void va_report(const char *msg_fmt,
> >>  		bool pass, bool xfail, bool skip, va_list va)
> >>  {
> >> -	char *prefix = skip ? "SKIP"
> >> -	                    : xfail ? (pass ? "XPASS" : "XFAIL")
> >> -	                            : (pass ? "PASS"  : "FAIL");
> >> +	const char *prefix = skip ? "SKIP"
> >> +				  : xfail ? (pass ? "XPASS" : "XFAIL")
> >> +				  : (pass ? "PASS"  : "FAIL");
> > 
> > Hmm, think I liked the old indentation better. (":" directly below the
> > matching "?")
> 
> Oh, right, thanks! That apparently happens when sending patches too
> early in the morning ;-)
> I'll wait for some more comments about whether to include
> "-Wwrite-strings" or not, then I'll respin this patch.

I vote turn it on globally, and then if a unit test is introduced where
it needs to be off, then that's the beauty of having it fine grained,
that unit test can modify its make target accordingly to remove it.

Thanks,
drew

Re: [kvm-unit-tests PATCH] Declare the prefix string variable in va_report() as const

[Paolo Bonzini]

On 27/06/2017 10:39, Thomas Huth wrote:
> Oh, right, thanks! That apparently happens when sending patches too
> early in the morning ;-)
> I'll wait for some more comments about whether to include
> "-Wwrite-strings" or not, then I'll respin this patch.

Yes, please do.

Paolo