Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
tree: Enable -Wwrite-strings
When compiling, this warning gives string literals the type const char[] to help catch accidental modification (which is undefined behaviour). There currently aren't any instances of this in flashrom, so let's enable this warning to keep it that way. This requires adding const qualifiers to the declarations of several variables that work with string literals.
Change-Id: I62d9bc194938a0c9a0e4cdff7ced8ea2e14cc1bc Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile M buspirate_spi.c M cli_classic.c M dmi.c M pony_spi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 6 files changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/34577/1
diff --git a/Makefile b/Makefile index 545e84a..1a20933 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ DIFF = diff PREFIX ?= /usr/local MANDIR ?= $(PREFIX)/share/man -CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes +CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes -Wwrite-strings EXPORTDIR ?= . RANLIB ?= ranlib PKG_CONFIG ?= pkg-config diff --git a/buspirate_spi.c b/buspirate_spi.c index fb066c2..bc6776b 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -112,7 +112,7 @@ return 0; }
-static int buspirate_wait_for_string(unsigned char *buf, char *key) +static int buspirate_wait_for_string(unsigned char *buf, const char *key) { unsigned int keylen = strlen(key); int ret; diff --git a/cli_classic.c b/cli_classic.c index 0591bfe..428efc1 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -77,7 +77,7 @@ exit(1); }
-static int check_filename(char *filename, char *type) +static int check_filename(char *filename, const char *type) { if (!filename || (filename[0] == '\0')) { fprintf(stderr, "Error: No %s file specified.\n", type); diff --git a/dmi.c b/dmi.c index ae90f7c..6132b47 100644 --- a/dmi.c +++ b/dmi.c @@ -69,7 +69,7 @@ static const struct { uint8_t type; uint8_t is_laptop; - char *name; + const char *name; } dmi_chassis_types[] = { {0x01, 2, "Other"}, {0x02, 2, "Unknown"}, diff --git a/pony_spi.c b/pony_spi.c index 6c03308..ed9d326 100644 --- a/pony_spi.c +++ b/pony_spi.c @@ -115,7 +115,7 @@ int i, data_out; char *arg = NULL; enum pony_type type = TYPE_SI_PROG; - char *name; + const char *name; int have_device = 0; int have_prog = 0;
diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 2c7966d..768401e 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -111,7 +111,7 @@ printf("\n"); }
-static void usage(char *argv[], char *error) +static void usage(char *argv[], const char *error) { if (error != NULL) { fprintf(stderr, "%s\n", error);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 1: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile File Makefile:
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile@33 PS2, Line 33: CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes -Wwrite-strings Add to Meson as well?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile File Makefile:
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile@33 PS2, Line 33: CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes -Wwrite-strings
Add to Meson as well?
Agreed. I wonder if there's a way to keep both build systems in sync.
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile File Makefile:
https://review.coreboot.org/c/flashrom/+/34577/2/Makefile@33 PS2, Line 33: CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes -Wwrite-strings
Agreed. I wonder if there's a way to keep both build systems in sync.
We could remove the Makefile and just use meson (ducks for cover)
Hello Angel Pons, Paul Menzel, David Hendricks, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/34577
to look at the new patch set (#3).
Change subject: tree: Enable -Wwrite-strings ......................................................................
tree: Enable -Wwrite-strings
When compiling, this warning gives string literals the type const char[] to help catch accidental modification (which is undefined behaviour). There currently aren't any instances of this in flashrom, so let's enable this warning to keep it that way. This requires adding const qualifiers to the declarations of several variables that work with string literals.
Change-Id: I62d9bc194938a0c9a0e4cdff7ced8ea2e14cc1bc Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile M buspirate_spi.c M cli_classic.c M dmi.c M meson.build M pony_spi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 7 files changed, 8 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/34577/3
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 4: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
Patch Set 4: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/34577 )
Change subject: tree: Enable -Wwrite-strings ......................................................................
tree: Enable -Wwrite-strings
When compiling, this warning gives string literals the type const char[] to help catch accidental modification (which is undefined behaviour). There currently aren't any instances of this in flashrom, so let's enable this warning to keep it that way. This requires adding const qualifiers to the declarations of several variables that work with string literals.
Change-Id: I62d9bc194938a0c9a0e4cdff7ced8ea2e14cc1bc Signed-off-by: Jacob Garber jgarber1@ualberta.ca Reviewed-on: https://review.coreboot.org/c/flashrom/+/34577 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: David Hendricks david.hendricks@gmail.com Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M Makefile M buspirate_spi.c M cli_classic.c M dmi.c M meson.build M pony_spi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 7 files changed, 8 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved
diff --git a/Makefile b/Makefile index 545e84a..1a20933 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ DIFF = diff PREFIX ?= /usr/local MANDIR ?= $(PREFIX)/share/man -CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes +CFLAGS ?= -Os -Wall -Wshadow -Wmissing-prototypes -Wwrite-strings EXPORTDIR ?= . RANLIB ?= ranlib PKG_CONFIG ?= pkg-config diff --git a/buspirate_spi.c b/buspirate_spi.c index 08cd04c..826bd84 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -113,7 +113,7 @@ return 0; }
-static int buspirate_wait_for_string(unsigned char *buf, char *key) +static int buspirate_wait_for_string(unsigned char *buf, const char *key) { unsigned int keylen = strlen(key); int ret; diff --git a/cli_classic.c b/cli_classic.c index d23298e..4844d1d 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -79,7 +79,7 @@ exit(1); }
-static int check_filename(char *filename, char *type) +static int check_filename(char *filename, const char *type) { if (!filename || (filename[0] == '\0')) { fprintf(stderr, "Error: No %s file specified.\n", type); diff --git a/dmi.c b/dmi.c index 9ec935a..c44221c 100644 --- a/dmi.c +++ b/dmi.c @@ -69,7 +69,7 @@ static const struct { uint8_t type; uint8_t is_laptop; - char *name; + const char *name; } dmi_chassis_types[] = { {0x01, 2, "Other"}, {0x02, 2, "Unknown"}, diff --git a/meson.build b/meson.build index e1b6c16..d778d71 100644 --- a/meson.build +++ b/meson.build @@ -11,8 +11,9 @@ lt_age = '0' lt_version = '@0@.@1@.@2@'.format(lt_current, lt_age, lt_revision)
-# hide some warnings +# hide/enable some warnings warning_flags = [ + '-Wwrite-strings', '-Wno-unused-parameter', '-Wno-sign-compare', '-Wno-address-of-packed-member', diff --git a/pony_spi.c b/pony_spi.c index 6c03308..ed9d326 100644 --- a/pony_spi.c +++ b/pony_spi.c @@ -115,7 +115,7 @@ int i, data_out; char *arg = NULL; enum pony_type type = TYPE_SI_PROG; - char *name; + const char *name; int have_device = 0; int have_prog = 0;
diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 2db0f62..6f13749 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -111,7 +111,7 @@ printf("\n"); }
-static void usage(char *argv[], char *error) +static void usage(char *argv[], const char *error) { if (error != NULL) { fprintf(stderr, "%s\n", error);