| Message ID | 1396691102-22904-14-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/05/2014 02:45 AM, Daniel Vetter wrote: > This is C standard hair-splitting, but afaict > - sum will be promoted to signed int in computation since > uint8_t fits > - signed overflow is undefined. > > No we need to add up an awful lot of bytes to actually make it ^^ Now > overflow. But I guess the real risk is gcc spotting this and going > bananas. Fix this by simply using unsigned in to force all computations > to use the well-defined unsigned behaviour. Seems reasonable... it also seems impossible (ha!) to break anything. Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> > Spotted by coverity. > > Cc: Russell King <rmk+kernel@arm.linux.org.uk> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Jean-Francois Moine <moinejf@free.fr> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c > index 48af5cac1902..ae2754760d77 100644 > --- a/drivers/gpu/drm/i2c/tda998x_drv.c > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c > @@ -568,7 +568,7 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) > > static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) > { > - uint8_t sum = 0; > + unsigned sum = 0; > > while (bytes--) > sum += *buf++; >
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index 48af5cac1902..ae2754760d77 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -568,7 +568,7 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data) static uint8_t tda998x_cksum(uint8_t *buf, size_t bytes) { - uint8_t sum = 0; + unsigned sum = 0; while (bytes--) sum += *buf++;
This is C standard hair-splitting, but afaict - sum will be promoted to signed int in computation since uint8_t fits - signed overflow is undefined. No we need to add up an awful lot of bytes to actually make it overflow. But I guess the real risk is gcc spotting this and going bananas. Fix this by simply using unsigned in to force all computations to use the well-defined unsigned behaviour. Spotted by coverity. Cc: Russell King <rmk+kernel@arm.linux.org.uk> Cc: Rob Clark <robdclark@gmail.com> Cc: Jean-Francois Moine <moinejf@free.fr> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i2c/tda998x_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)