From: Arnd Bergmann
Sent: 29 April 2020 20:02 gcc-10 points out a few instances of suspicious integer arithmetic leading to value truncation:
sound/isa/opti9xx/opti92x-ad1848.c: In function 'snd_opti9xx_configure': sound/isa/opti9xx/opti92x-ad1848.c:322:43: error: overflow in conversion from 'int' to 'unsigned char' changes value from '(int)snd_opti9xx_read(chip, 3) & -256 | 240' to '240' [-Werror=overflow] 322 | (snd_opti9xx_read(chip, reg) & ~(mask)) | ((value) & (mask))) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~ sound/isa/opti9xx/opti92x-ad1848.c:351:3: note: in expansion of macro 'snd_opti9xx_write_mask' 351 | snd_opti9xx_write_mask(chip, OPTi9XX_MC_REG(3), 0xf0, 0xff); | ^~~~~~~~~~~~~~~~~~~~~~ sound/isa/opti9xx/miro.c: In function 'snd_miro_configure': sound/isa/opti9xx/miro.c:873:40: error: overflow in conversion from 'int' to 'unsigned char' changes value from '(int)snd_miro_read(chip, 3) & -256 | 240' to '240' [-Werror=overflow] 873 | (snd_miro_read(chip, reg) & ~(mask)) | ((value) & (mask))) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~ sound/isa/opti9xx/miro.c:1010:3: note: in expansion of macro 'snd_miro_write_mask' 1010 | snd_miro_write_mask(chip, OPTi9XX_MC_REG(3), 0xf0, 0xff); | ^~~~~~~~~~~~~~~~~~~
These are all harmless here as only the low 8 bit are passed down anyway. Change the macros to inline functions to make the code more readable and also avoid the warning.
Strictly speaking those functions also need locking to make the read/write pair atomic, but it seems unlikely that anyone would still run into that issue.
Fixes: 1841f613fd2e ("[ALSA] Add snd-miro driver") Signed-off-by: Arnd Bergmann arnd@arndb.de
sound/isa/opti9xx/miro.c | 9 ++++++--- sound/isa/opti9xx/opti92x-ad1848.c | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/isa/opti9xx/miro.c b/sound/isa/opti9xx/miro.c index e764816a8f7a..b039429e6871 100644 --- a/sound/isa/opti9xx/miro.c +++ b/sound/isa/opti9xx/miro.c @@ -867,10 +867,13 @@ static void snd_miro_write(struct snd_miro *chip, unsigned char reg, spin_unlock_irqrestore(&chip->lock, flags); }
+static inline void snd_miro_write_mask(struct snd_miro *chip,
unsigned char reg, unsigned char value, unsigned char mask)+{
- unsigned char oldval = snd_miro_read(chip, reg);
-#define snd_miro_write_mask(chip, reg, value, mask) \
- snd_miro_write(chip, reg, \
(snd_miro_read(chip, reg) & ~(mask)) | ((value) & (mask)))
- snd_miro_write(chip, reg, (oldval & ~mask) | (value & mask));
+}
Isn't that likely to add additional masking with 0xff at the call sites? You will probably get better code if the arguments are 'unsigned int'.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)