| Message ID | 1498651457-19688-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28.06.2017 14:04, Thomas Huth wrote: > So we make sure that we do not accidentially write to constant "accidentally" (I wouldn't know if Thunderbird wouldn't tell me ;) ) > strings. Also add some missing "const" qualifiers in the code to > avoid that we get compiler warnings now. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > This patch supersedes my previous patch "Declare the prefix string > variable in va_report() as const" > > Makefile | 2 +- > lib/report.c | 6 +++--- > x86/msr.c | 4 ++-- > x86/pmu.c | 2 +- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index 933b9f0..e79cf93 100644 > --- a/Makefile > +++ b/Makefile > @@ -51,7 +51,7 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) > > CFLAGS += -g > -CFLAGS += $(autodepend-flags) -Wall -Werror > +CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror I assume this option has been around for quite some while, so no reasonable gcc will spit fire. Reviewed-by: David Hildenbrand <david@redhat.com>
On 28.06.2017 19:27, David Hildenbrand wrote: > On 28.06.2017 14:04, Thomas Huth wrote: >> So we make sure that we do not accidentially write to constant > > "accidentally" (I wouldn't know if Thunderbird wouldn't tell me ;) ) Oh, ok. Paolo, Radim, could you please fix it when picking up the patch? >> strings. Also add some missing "const" qualifiers in the code to >> avoid that we get compiler warnings now. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> This patch supersedes my previous patch "Declare the prefix string >> variable in va_report() as const" >> >> Makefile | 2 +- >> lib/report.c | 6 +++--- >> x86/msr.c | 4 ++-- >> x86/pmu.c | 2 +- >> 4 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 933b9f0..e79cf93 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -51,7 +51,7 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ >> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) >> >> CFLAGS += -g >> -CFLAGS += $(autodepend-flags) -Wall -Werror >> +CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror > > > I assume this option has been around for quite some while, so no > reasonable gcc will spit fire. A very quick search revealed that it is at least 14 years old and GCC 4.4 already contained it. Likely even earlier versions. I think we do not really support older versions anymore, so that should be ok. > Reviewed-by: David Hildenbrand <david@redhat.com> Thanks! Thomas
On 28/06/2017 22:10, Thomas Huth wrote: > On 28.06.2017 19:27, David Hildenbrand wrote: >> On 28.06.2017 14:04, Thomas Huth wrote: >>> So we make sure that we do not accidentially write to constant >> >> "accidentally" (I wouldn't know if Thunderbird wouldn't tell me ;) ) > > Oh, ok. Paolo, Radim, could you please fix it when picking up the patch? Sure. >> I assume this option has been around for quite some while, so no >> reasonable gcc will spit fire. > > A very quick search revealed that it is at least 14 years old and GCC > 4.4 already contained it. Likely even earlier versions. I think we do > not really support older versions anymore, so that should be ok. Yes, I think I've used it in GCC 2.x or so. :) Paolo >> Reviewed-by: David Hildenbrand <david@redhat.com> > > Thanks! > > Thomas >
diff --git a/Makefile b/Makefile index 933b9f0..e79cf93 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) CFLAGS += -g -CFLAGS += $(autodepend-flags) -Wall -Werror +CFLAGS += $(autodepend-flags) -Wall -Wwrite-strings -Werror frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "") fnostack_protector := $(call cc-option, -fno-stack-protector, "") diff --git a/lib/report.c b/lib/report.c index b002d21..5da27ab 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); diff --git a/x86/msr.c b/x86/msr.c index 91351a3..ffc24b1 100644 --- a/x86/msr.c +++ b/x86/msr.c @@ -6,7 +6,7 @@ struct msr_info { int index; - char *name; + const char *name; struct tc { int valid; unsigned long long value; @@ -78,7 +78,7 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l { unsigned long long r = 0; int index; - char *sptr; + const char *sptr; if ((index = find_msr_info(msr_index)) != -1) { sptr = msr_info[index].name; } else { diff --git a/x86/pmu.c b/x86/pmu.c index c689800..a0238dc 100644 --- a/x86/pmu.c +++ b/x86/pmu.c @@ -73,7 +73,7 @@ union cpuid10_edx { } edx; struct pmu_event { - char *name; + const char *name; uint32_t unit_sel; int min; int max;
So we make sure that we do not accidentially write to constant strings. Also add some missing "const" qualifiers in the code to avoid that we get compiler warnings now. Signed-off-by: Thomas Huth <thuth@redhat.com> --- This patch supersedes my previous patch "Declare the prefix string variable in va_report() as const" Makefile | 2 +- lib/report.c | 6 +++--- x86/msr.c | 4 ++-- x86/pmu.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-)