On Tue, 2017-09-05 at 13:51 +0200, Marcel Holtmann wrote:
> Hi Bastien,
>
> > GCC's "format-nonliteral" security check is enabled as an error in
> > recent versions of Fedora. Given the reduced scope of use, mark the
> > error as ignorable through pragma.
> >
> > tools/csr_usb.c: In function 'read_value':
> > tools/csr_usb.c:82:2: error: format not a string literal, argument
> > types not checked [-Werror=format-nonliteral]
> > n = fscanf(file, format, &value);
> > ^
> > ---
> > tools/csr_usb.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/csr_usb.c b/tools/csr_usb.c
> > index a1d7324f7..33e9968a2 100644
> > --- a/tools/csr_usb.c
> > +++ b/tools/csr_usb.c
> > @@ -67,6 +67,8 @@ struct usbfs_bulktransfer {
> > #define USBFS_IOCTL_CLAIMINTF _IOR('U', 15, unsigned int)
> > #define USBFS_IOCTL_RELEASEINTF _IOR('U', 16, unsigned int)
> >
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > static int read_value(const char *name, const char *attr, const
> > char *format)
> > {
> > char path[PATH_MAX];
> > @@ -88,6 +90,7 @@ static int read_value(const char *name, const
> > char *attr, const char *format)
> > fclose(file);
> > return value;
> > }
> > +#pragma GCC diagnostic pop
>
> can’t we just use __attribute__((format)) somehow to declare this a
> format specify?
Sorry, I'm not sure I understand.
gcc already knows that this is a format, but because the format changes
depending on the caller, it cannot actually assert whether the format
is safe for the arguments passed.
So this would work:
vendor = read_value(name, "idVendor", "%04x");
But the compiler can't see that:
vendor = read_value(name, "idVendor", "%s");
would fail at compile time.
Would you prefer something like this (untested, but should work):
diff --git a/tools/csr_usb.c b/tools/csr_usb.c
index a1d7324f7..c5a39899a 100644
--- a/tools/csr_usb.c
+++ b/tools/csr_usb.c
@@ -67,7 +67,7 @@ struct usbfs_bulktransfer {
#define USBFS_IOCTL_CLAIMINTF _IOR('U', 15, unsigned int)
#define USBFS_IOCTL_RELEASEINTF _IOR('U', 16, unsigned int)
-static int read_value(const char *name, const char *attr, const char *format)
+static int read_value(const char *name, const char *attr, bool hex_number)
{
char path[PATH_MAX];
FILE *file;
@@ -79,7 +79,7 @@ static int read_value(const char *name, const char *attr, const char *format)
if (!file)
return -1;
- n = fscanf(file, format, &value);
+ n = fscanf(file, hex_number ? "%d" : "%04x", &value);
if (n != 1) {
fclose(file);
return -1;
@@ -94,21 +94,21 @@ static char *check_device(const char *name)
char path[PATH_MAX];
int busnum, devnum, vendor, product;
- busnum = read_value(name, "busnum", "%d");
+ busnum = read_value(name, "busnum", false);
if (busnum < 0)
return NULL;
- devnum = read_value(name, "devnum", "%d");
+ devnum = read_value(name, "devnum", false);
if (devnum < 0)
return NULL;
snprintf(path, sizeof(path), "/dev/bus/usb/%03u/%03u", busnum, devnum);
- vendor = read_value(name, "idVendor", "%04x");
+ vendor = read_value(name, "idVendor", true);
if (vendor < 0)
return NULL;
- product = read_value(name, "idProduct", "%04x");
+ product = read_value(name, "idProduct", true);
if (product < 0)
return NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
