diff mbox series

Fix FTBFS with -Werror=format-overflow

Message ID 20180125131455.14466-1-frediz@linux.vnet.ibm.com
State Changes Requested
Headers show
Series Fix FTBFS with -Werror=format-overflow | expand

Commit Message

Frédéric Bonnard Jan. 25, 2018, 1:14 p.m. UTC
i2c.c fails to compile with gcc7 and -Werror=format-overflow used in
Debian Unstable and Ubuntu 18.04 :

i2c.c: In function ‘i2c_init’:
i2c.c:211:15: error: ‘%s’ directive writing up to 255 bytes into a
region of size 236 [-Werror=format-overflow=]

We just make sure we never write more than what the destination can
store.

Signed-off-by: Frédéric Bonnard <frediz@linux.vnet.ibm.com>
---
 external/opal-prd/i2c.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Andrew Donnellan Jan. 30, 2018, 2:28 a.m. UTC | #1
On 26/01/18 00:14, Frédéric Bonnard wrote:
> i2c.c fails to compile with gcc7 and -Werror=format-overflow used in
> Debian Unstable and Ubuntu 18.04 :
> 
> i2c.c: In function ‘i2c_init’:
> i2c.c:211:15: error: ‘%s’ directive writing up to 255 bytes into a
> region of size 236 [-Werror=format-overflow=]
> 
> We just make sure we never write more than what the destination can
> store.
> 
> Signed-off-by: Frédéric Bonnard <frediz@linux.vnet.ibm.com>
> ---
>   external/opal-prd/i2c.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/external/opal-prd/i2c.c b/external/opal-prd/i2c.c
> index ba4b3c85..dca303e4 100644
> --- a/external/opal-prd/i2c.c
> +++ b/external/opal-prd/i2c.c
> @@ -234,7 +234,13 @@ void i2c_init(void)
>   			continue;
>   
>   		/* Get bus name */
> -		sprintf(dpath, SYSFS "/class/i2c-dev/%s/name", devent->d_name);
> +		if (snprintf(dpath, NAME_MAX, SYSFS "/class/i2c-dev/%s/name",
> +					devent->d_name) < 0 ) {

If the string is too long, snprintf() will truncate and return number of 
bytes written, not a negative error code.

> +			pr_log(LOG_ERR, "I2C: bus name longer than NAME_MAX : "

Strictly speaking, the bus name doesn't have to be longer than NAME_MAX, 
it's the entire path.

I suspect that dpath should actually be of length PATH_MAX...


Andrew

> +					SYSFS "/class/i2c-dev/%s/name",
> +					devent->d_name);
> +			return;
> +		}
>   		f = fopen(dpath, "r");
>   		if (!f) {
>   			pr_log(LOG_NOTICE, "I2C: Can't open %s: %m, skipping.",
>
Frédéric Bonnard Jan. 31, 2018, 12:48 p.m. UTC | #2
Hi Andrew,

On Tue, 30 Jan 2018 13:28:24 +1100, Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote:
> On 26/01/18 00:14, Frédéric Bonnard wrote:
> > i2c.c fails to compile with gcc7 and -Werror=format-overflow used in
> > Debian Unstable and Ubuntu 18.04 :
> > 
> > i2c.c: In function ‘i2c_init’:
> > i2c.c:211:15: error: ‘%s’ directive writing up to 255 bytes into a
> > region of size 236 [-Werror=format-overflow=]
> > 
> > We just make sure we never write more than what the destination can
> > store.
> > 
> > Signed-off-by: Frédéric Bonnard <frediz@linux.vnet.ibm.com>
> > ---
> >   external/opal-prd/i2c.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/external/opal-prd/i2c.c b/external/opal-prd/i2c.c
> > index ba4b3c85..dca303e4 100644
> > --- a/external/opal-prd/i2c.c
> > +++ b/external/opal-prd/i2c.c
> > @@ -234,7 +234,13 @@ void i2c_init(void)
> >   			continue;
> >   
> >   		/* Get bus name */
> > -		sprintf(dpath, SYSFS "/class/i2c-dev/%s/name", devent->d_name);
> > +		if (snprintf(dpath, NAME_MAX, SYSFS "/class/i2c-dev/%s/name",
> > +					devent->d_name) < 0 ) {
> 
> If the string is too long, snprintf() will truncate and return number of 
> bytes written, not a negative error code.

indeed.. what was I thinking.

> > +			pr_log(LOG_ERR, "I2C: bus name longer than NAME_MAX : "
> 
> Strictly speaking, the bus name doesn't have to be longer than NAME_MAX, 
> it's the entire path.
> 
> I suspect that dpath should actually be of length PATH_MAX...

That would make much more sense actually. Thanks for your help.

F.

> 
> Andrew
> 
> > +					SYSFS "/class/i2c-dev/%s/name",
> > +					devent->d_name);
> > +			return;
> > +		}
> >   		f = fopen(dpath, "r");
> >   		if (!f) {
> >   			pr_log(LOG_NOTICE, "I2C: Can't open %s: %m, skipping.",
> > 
> 
> -- 
> Andrew Donnellan              OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com  IBM Australia Limited
diff mbox series

Patch

diff --git a/external/opal-prd/i2c.c b/external/opal-prd/i2c.c
index ba4b3c85..dca303e4 100644
--- a/external/opal-prd/i2c.c
+++ b/external/opal-prd/i2c.c
@@ -234,7 +234,13 @@  void i2c_init(void)
 			continue;
 
 		/* Get bus name */
-		sprintf(dpath, SYSFS "/class/i2c-dev/%s/name", devent->d_name);
+		if (snprintf(dpath, NAME_MAX, SYSFS "/class/i2c-dev/%s/name",
+					devent->d_name) < 0 ) {
+			pr_log(LOG_ERR, "I2C: bus name longer than NAME_MAX : "
+					SYSFS "/class/i2c-dev/%s/name",
+					devent->d_name);
+			return;
+		}
 		f = fopen(dpath, "r");
 		if (!f) {
 			pr_log(LOG_NOTICE, "I2C: Can't open %s: %m, skipping.",