| Message ID | 20180621205854.13765-1-rosenp@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel] procd: Fix compile error with GCC8 | expand |
On 21/06/18 22:58, Rosen Penev wrote: > error: ‘%d’ directive output may be truncated writing between 2 and 10 > bytes into a region of size 3 [-Werror=format-truncation=] > snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); > ^~~ > note: directive argument in the range [0, 2147483647] > snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); > ^~~~~ > note: ‘snprintf’ output between 3 and 11 bytes into a destination of size > 3 > snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > watchdog.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/watchdog.c b/watchdog.c > index ec10ba6..efe30f1 100644 > --- a/watchdog.c > +++ b/watchdog.c > @@ -144,8 +144,9 @@ char* watchdog_fd(void) > { > static char fd_buf[3]; > > - if (wdt_fd < 0) > + if (wdt_fd < 0 || wdt_fd > 99) > return NULL; > + > snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); imho the %d should be changed to %ld or something similar. > > return fd_buf;
Citeren John Crispin <john@phrozen.org>: > On 21/06/18 22:58, Rosen Penev wrote: >> error: ‘%d’ directive output may be truncated writing between 2 and 10 >> bytes into a region of size 3 [-Werror=format-truncation=] >> snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); >> ^~~ >> note: directive argument in the range [0, 2147483647] >> snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); >> ^~~~~ >> note: ‘snprintf’ output between 3 and 11 bytes into a destination of size >> 3 >> snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> watchdog.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/watchdog.c b/watchdog.c >> index ec10ba6..efe30f1 100644 >> --- a/watchdog.c >> +++ b/watchdog.c >> @@ -144,8 +144,9 @@ char* watchdog_fd(void) >> { >> static char fd_buf[3]; >> - if (wdt_fd < 0) >> + if (wdt_fd < 0 || wdt_fd > 99) >> return NULL; >> + >> snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); > imho the %d should be changed to %ld or something similar. That's not the problem here. The fd_buf is only large enough to store values between 0 and 99 (inclusive). Although this will fix the compilation warning/error, I'm slightly worried that this may confuse callers when the filedescriptor is > 99. Before this patch, you'd get a truncated value (the first two digits), after there will be no difference between an error condition (wdt_fd < 0) or a filedescriptor that is too large to fit in the static fd_buf. The wdt_fd is defined as an 'int', so %d is appropriate here. >> return fd_buf; > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/listinfo/openwrt-devel
diff --git a/watchdog.c b/watchdog.c index ec10ba6..efe30f1 100644 --- a/watchdog.c +++ b/watchdog.c @@ -144,8 +144,9 @@ char* watchdog_fd(void) { static char fd_buf[3]; - if (wdt_fd < 0) + if (wdt_fd < 0 || wdt_fd > 99) return NULL; + snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); return fd_buf;
error: ‘%d’ directive output may be truncated writing between 2 and 10 bytes into a region of size 3 [-Werror=format-truncation=] snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); ^~~ note: directive argument in the range [0, 2147483647] snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); ^~~~~ note: ‘snprintf’ output between 3 and 11 bytes into a destination of size 3 snprintf(fd_buf, sizeof(fd_buf), "%d", wdt_fd); Signed-off-by: Rosen Penev <rosenp@gmail.com> --- watchdog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)