| Message ID | 1498540173-3030-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); > >
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
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
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
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);
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(-)