Richard W.M. Jones
2017-Feb-14 15:02 UTC
[Libguestfs] [PATCH 1/2] GCC 7: Add __attribute__((noreturn)) to some usage functions which call exit.
This happens with GCC 7.0.1. The errors were all of the form:
qemu-speed-test.c: In function 'main':
qemu-speed-test.c:153:7: error: this statement may fall through
[-Werror=implicit-fallthrough=]
usage (EXIT_SUCCESS);
^~~~~~~~~~~~~~~~~~~~
qemu-speed-test.c:155:5: note: here
default:
^~~~~~~
---
builder/index-validate.c | 2 +-
utils/boot-analysis/boot-analysis.c | 2 +-
utils/boot-benchmark/boot-benchmark.c | 2 +-
utils/qemu-boot/qemu-boot.c | 2 +-
utils/qemu-speed-test/qemu-speed-test.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builder/index-validate.c b/builder/index-validate.c
index d3912f3..33086b2 100644
--- a/builder/index-validate.c
+++ b/builder/index-validate.c
@@ -38,7 +38,7 @@
extern int do_parse (struct parse_context *context, FILE *in);
-static void
+static void __attribute__((noreturn))
usage (int exit_status)
{
printf ("%s index\n", getprogname ());
diff --git a/utils/boot-analysis/boot-analysis.c
b/utils/boot-analysis/boot-analysis.c
index 04b3bdd..1bec9a5 100644
--- a/utils/boot-analysis/boot-analysis.c
+++ b/utils/boot-analysis/boot-analysis.c
@@ -88,7 +88,7 @@ static void print_longest_to_shortest (void);
static void free_pass_data (void);
static void free_final_timeline (void);
-static void
+static void __attribute__((noreturn))
usage (int exitcode)
{
guestfs_h *g;
diff --git a/utils/boot-benchmark/boot-benchmark.c
b/utils/boot-benchmark/boot-benchmark.c
index 73da722..4af3943 100644
--- a/utils/boot-benchmark/boot-benchmark.c
+++ b/utils/boot-benchmark/boot-benchmark.c
@@ -51,7 +51,7 @@ static void run_test (void);
static guestfs_h *create_handle (void);
static void add_drive (guestfs_h *g);
-static void
+static void __attribute__((noreturn))
usage (int exitcode)
{
guestfs_h *g;
diff --git a/utils/qemu-boot/qemu-boot.c b/utils/qemu-boot/qemu-boot.c
index 1551a1a..6881835 100644
--- a/utils/qemu-boot/qemu-boot.c
+++ b/utils/qemu-boot/qemu-boot.c
@@ -74,7 +74,7 @@ static void run_test (size_t P);
static void *start_thread (void *thread_data_vp);
static void message_callback (guestfs_h *g, void *opaque, uint64_t event, int
event_handle, int flags, const char *buf, size_t buf_len, const uint64_t *array,
size_t array_len);
-static void
+static void __attribute__((noreturn))
usage (int exitcode)
{
fprintf (stderr,
diff --git a/utils/qemu-speed-test/qemu-speed-test.c
b/utils/qemu-speed-test/qemu-speed-test.c
index 54875fa..5aa663c 100644
--- a/utils/qemu-speed-test/qemu-speed-test.c
+++ b/utils/qemu-speed-test/qemu-speed-test.c
@@ -66,7 +66,7 @@ reset_default_tests (int *flag)
}
}
-static void
+static void __attribute__((noreturn))
usage (int exitcode)
{
fprintf (stderr,
--
2.10.2
Richard W.M. Jones
2017-Feb-14 15:02 UTC
[Libguestfs] [PATCH 2/2] GCC 7: Allocate sufficient space for sprintf output.
GCC 7.0.1 can determine if there is likely to be sufficient space in
the output buffer when using sprintf/snprintf, based on the format
string.
The errors were all either of this form:
bindtests.c:717:29: error: '%zu' directive output may be truncated
writing between 1 and 19 bytes into a region of size 16
[-Werror=format-truncation=]
snprintf (strs[i], 16, "%zu", i);
^~~
bindtests.c:717:28: note: directive argument in the range [0,
2305843009213693951]
snprintf (strs[i], 16, "%zu", i);
^~~~~
or this form:
sync.c: In function 'fsync_devices':
sync.c:108:50: error: '%s' directive output may be truncated writing up
to 255 bytes into a region of size 251 [-Werror=format-truncation=]
snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name);
^~
Fixed by converting these into dynamic allocation, or making the
output buffer larger, whichever was easier.
---
cat/filesystems.c | 2 +-
daemon/9p.c | 10 +++++++---
daemon/debug.c | 12 ++++++++++--
daemon/devsparts.c | 16 ++++++++++++----
daemon/sync.c | 7 +++++--
generator/bindtests.ml | 12 ++++++------
6 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/cat/filesystems.c b/cat/filesystems.c
index 1036c6f..4264b0f 100644
--- a/cat/filesystems.c
+++ b/cat/filesystems.c
@@ -866,7 +866,7 @@ write_row (const char *name, const char *type,
size_t len = 0;
char hum[LONGEST_HUMAN_READABLE];
char num[256];
- char mbr_id_str[3];
+ char mbr_id_str[9];
if ((columns & COLUMN_NAME))
strings[len++] = name;
diff --git a/daemon/9p.c b/daemon/9p.c
index a9e36d1..f72c8dd 100644
--- a/daemon/9p.c
+++ b/daemon/9p.c
@@ -71,9 +71,13 @@ do_list_9p (void)
if (d == NULL) break;
if (STRPREFIX (d->d_name, "virtio")) {
- char mount_tag_path[256];
- snprintf (mount_tag_path, sizeof mount_tag_path,
- BUS_PATH "/%s/mount_tag", d->d_name);
+ CLEANUP_FREE char *mount_tag_path;
+ if (asprintf (&mount_tag_path, BUS_PATH "/%s/mount_tag",
+ d->d_name) == -1) {
+ reply_with_perror ("asprintf");
+ closedir (dir);
+ return NULL;
+ }
/* A bit unclear, but it looks like the virtio transport allows
* the mount tag length to be unlimited (or up to 65536 bytes).
diff --git a/daemon/debug.c b/daemon/debug.c
index 06f0729..b18d87c 100644
--- a/daemon/debug.c
+++ b/daemon/debug.c
@@ -161,7 +161,7 @@ debug_fds (const char *subcmd, size_t argc, char *const
*const argv)
FILE *fp;
DIR *dir;
struct dirent *d;
- char fname[256], link[256];
+ char link[256];
struct stat statbuf;
fp = open_memstream (&out, &size);
@@ -178,10 +178,18 @@ debug_fds (const char *subcmd, size_t argc, char *const
*const argv)
}
while ((d = readdir (dir)) != NULL) {
+ CLEANUP_FREE char *fname = NULL;
+
if (STREQ (d->d_name, ".") || STREQ (d->d_name,
".."))
continue;
- snprintf (fname, sizeof fname, "/proc/self/fd/%s", d->d_name);
+ if (asprintf (&fname, "/proc/self/fd/%s", d->d_name) ==
-1) {
+ reply_with_perror ("asprintf");
+ fclose (fp);
+ free (out);
+ closedir (dir);
+ return NULL;
+ }
r = lstat (fname, &statbuf);
if (r == -1) {
diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index f15d2c3..96e867f 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -43,7 +43,6 @@ foreach_block_device (block_dev_func_t func, bool return_md)
DIR *dir;
int err = 0;
struct dirent *d;
- char dev_path[256];
int fd;
dir = opendir ("/sys/block");
@@ -64,7 +63,12 @@ foreach_block_device (block_dev_func_t func, bool return_md)
STREQLEN (d->d_name, "sr", 2) ||
(return_md &&
STREQLEN (d->d_name, "md", 2) && c_isdigit
(d->d_name[2]))) {
- snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name);
+ CLEANUP_FREE char *dev_path;
+ if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) {
+ reply_with_perror ("asprintf");
+ closedir (dir);
+ return NULL;
+ }
/* Ignore the root device. */
if (is_root_device (dev_path))
@@ -167,8 +171,12 @@ add_partitions (const char *device, struct stringsbuf *r)
struct dirent *d;
while ((d = readdir (dir)) != NULL) {
if (STREQLEN (d->d_name, device, strlen (device))) {
- char part[256];
- snprintf (part, sizeof part, "/dev/%s", d->d_name);
+ CLEANUP_FREE char *part;
+ if (asprintf (&part, "/dev/%s", d->d_name) == -1) {
+ perror ("asprintf");
+ closedir (dir);
+ return -1;
+ }
if (add_string (r, part) == -1) {
closedir (dir);
diff --git a/daemon/sync.c b/daemon/sync.c
index 581fa0f..b64f6e3 100644
--- a/daemon/sync.c
+++ b/daemon/sync.c
@@ -86,7 +86,6 @@ fsync_devices (void)
{
DIR *dir;
struct dirent *d;
- char dev_path[256];
int fd;
dir = opendir ("/sys/block");
@@ -105,7 +104,11 @@ fsync_devices (void)
STREQLEN (d->d_name, "ubd", 3) ||
STREQLEN (d->d_name, "vd", 2) ||
STREQLEN (d->d_name, "sr", 2)) {
- snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name);
+ CLEANUP_FREE char *dev_path;
+ if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) {
+ perror ("asprintf");
+ continue;
+ }
/* Ignore the root device. */
if (is_root_device (dev_path))
diff --git a/generator/bindtests.ml b/generator/bindtests.ml
index 22d0c37..2fde425 100644
--- a/generator/bindtests.ml
+++ b/generator/bindtests.ml
@@ -258,8 +258,8 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t
i)
pr " }\n";
pr " strs = safe_malloc (g, (n+1) * sizeof (char
*));\n";
pr " for (i = 0; i < n; ++i) {\n";
- pr " strs[i] = safe_malloc (g, 16);\n";
- pr " snprintf (strs[i], 16, \"%%zu\",
i);\n";
+ pr " strs[i] = safe_malloc (g, 20);\n";
+ pr " snprintf (strs[i], 20, \"%%zu\",
i);\n";
pr " }\n";
pr " strs[n] = NULL;\n";
pr " return strs;\n"
@@ -290,10 +290,10 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv,
size_t i)
pr " }\n";
pr " strs = safe_malloc (g, (n*2+1) * sizeof
(*strs));\n";
pr " for (i = 0; i < n; ++i) {\n";
- pr " strs[i*2] = safe_malloc (g, 16);\n";
- pr " strs[i*2+1] = safe_malloc (g, 16);\n";
- pr " snprintf (strs[i*2], 16, \"%%zu\",
i);\n";
- pr " snprintf (strs[i*2+1], 16, \"%%zu\",
i);\n";
+ pr " strs[i*2] = safe_malloc (g, 20);\n";
+ pr " strs[i*2+1] = safe_malloc (g, 20);\n";
+ pr " snprintf (strs[i*2], 20, \"%%zu\",
i);\n";
+ pr " snprintf (strs[i*2+1], 20, \"%%zu\",
i);\n";
pr " }\n";
pr " strs[n*2] = NULL;\n";
pr " return strs;\n"
--
2.10.2
Daniel P. Berrange
2017-Feb-14 15:14 UTC
Re: [Libguestfs] [PATCH 2/2] GCC 7: Allocate sufficient space for sprintf output.
On Tue, Feb 14, 2017 at 03:02:14PM +0000, Richard W.M. Jones wrote:> GCC 7.0.1 can determine if there is likely to be sufficient space in > the output buffer when using sprintf/snprintf, based on the format > string. > > The errors were all either of this form: > > bindtests.c:717:29: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=] > snprintf (strs[i], 16, "%zu", i); > ^~~ > bindtests.c:717:28: note: directive argument in the range [0, 2305843009213693951] > snprintf (strs[i], 16, "%zu", i); > ^~~~~ > > or this form: > > sync.c: In function 'fsync_devices': > sync.c:108:50: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 251 [-Werror=format-truncation=] > snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name); > ^~ > > Fixed by converting these into dynamic allocation, or making the > output buffer larger, whichever was easier. > --- > cat/filesystems.c | 2 +- > daemon/9p.c | 10 +++++++--- > daemon/debug.c | 12 ++++++++++-- > daemon/devsparts.c | 16 ++++++++++++---- > daemon/sync.c | 7 +++++-- > generator/bindtests.ml | 12 ++++++------ > 6 files changed, 41 insertions(+), 18 deletions(-) > > diff --git a/cat/filesystems.c b/cat/filesystems.c > index 1036c6f..4264b0f 100644 > --- a/cat/filesystems.c > +++ b/cat/filesystems.c > @@ -866,7 +866,7 @@ write_row (const char *name, const char *type, > size_t len = 0; > char hum[LONGEST_HUMAN_READABLE]; > char num[256]; > - char mbr_id_str[3]; > + char mbr_id_str[9];[snip]> @@ -258,8 +258,8 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i) > pr " }\n"; > pr " strs = safe_malloc (g, (n+1) * sizeof (char *));\n"; > pr " for (i = 0; i < n; ++i) {\n"; > - pr " strs[i] = safe_malloc (g, 16);\n"; > - pr " snprintf (strs[i], 16, \"%%zu\", i);\n"; > + pr " strs[i] = safe_malloc (g, 20);\n"; > + pr " snprintf (strs[i], 20, \"%%zu\", i);\n"; > pr " }\n"; > pr " strs[n] = NULL;\n"; > pr " return strs;\n" > @@ -290,10 +290,10 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i) > pr " }\n"; > pr " strs = safe_malloc (g, (n*2+1) * sizeof (*strs));\n"; > pr " for (i = 0; i < n; ++i) {\n"; > - pr " strs[i*2] = safe_malloc (g, 16);\n"; > - pr " strs[i*2+1] = safe_malloc (g, 16);\n"; > - pr " snprintf (strs[i*2], 16, \"%%zu\", i);\n"; > - pr " snprintf (strs[i*2+1], 16, \"%%zu\", i);\n"; > + pr " strs[i*2] = safe_malloc (g, 20);\n"; > + pr " strs[i*2+1] = safe_malloc (g, 20);\n"; > + pr " snprintf (strs[i*2], 20, \"%%zu\", i);\n"; > + pr " snprintf (strs[i*2+1], 20, \"%%zu\", i);\n"; > pr " }\n"; > pr " strs[n*2] = NULL;\n"; > pr " return strs;\n"FWIW, for cases where you really do want to print an integer into a fixed length buffer, then gnulib has a useful macro to let you automatically determine the correct buffer size at compile time. size_t someval; char buf[INT_BUFSIZE_BOUND(someval)] snprintf(buf, ARRAY_CARDINALITY(buf), "%zu", someval) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
Apparently Analagous Threads
- [PATCH v2 0/2] GCC 7: Misc fixes
- [PATCH 2/2] GCC 7: Allocate sufficient space for sprintf output.
- [PATCH v2 2/2] GCC 7: Allocate sufficient space for sprintf output.
- [PATCH 1/2] daemon: Fix part-to-dev when the partition name includes p<N>.
- [PATCH 0/7 v2] Fix and workaround for qcow2 issues in qemu causing data corruption.