Hi Jean,

On Tue, Dec 13, 2016 at 10:48:29AM +0100, Jean Delvare wrote:
> Hi Guenter,
> 
> On Sun,  4 Dec 2016 20:55:39 -0800, Guenter Roeck wrote:
> > Writes into limit attributes can overflow due to additions and
> > multiplications with unchecked parameters.
> > 
> > Signed-off-by: Guenter Roeck <[email protected]>
> > ---
> >  drivers/hwmon/gl518sm.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/hwmon/gl518sm.c b/drivers/hwmon/gl518sm.c
> > index 0212c8317bca..0a11a550c2a2 100644
> > --- a/drivers/hwmon/gl518sm.c
> > +++ b/drivers/hwmon/gl518sm.c
> > @@ -86,9 +86,8 @@ enum chips { gl518sm_r00, gl518sm_r80 };
> >  #define BOOL_FROM_REG(val) ((val) ? 0 : 1)
> >  #define BOOL_TO_REG(val)   ((val) ? 0 : 1)
> >  
> > -#define TEMP_TO_REG(val)   clamp_val(((((val) < 0 ? \
> > -                           (val) - 500 : \
> > -                           (val) + 500) / 1000) + 119), 0, 255)
> > +#define TEMP_TO_REG(val) (DIV_ROUND_CLOSEST(clamp_val(val, -119000, 
> > 136000), \
> > +                                       1000) + 119)
> >  #define TEMP_FROM_REG(val) (((val) - 119) * 1000)
> >  
> >  static inline u8 FAN_TO_REG(long rpm, int div)
> > @@ -101,10 +100,11 @@ static inline u8 FAN_TO_REG(long rpm, int div)
> >  }
> >  #define FAN_FROM_REG(val, div)     ((val) == 0 ? 0 : (480000 / ((val) * 
> > (div))))
> >  
> > -#define IN_TO_REG(val)             clamp_val((((val) + 9) / 19), 0, 255)
> > +#define IN_TO_REG(val)     DIV_ROUND_CLOSEST(clamp_val(val, 0, 255 * 19), 
> > 19)
> >  #define IN_FROM_REG(val)   ((val) * 19)
> >  
> > -#define VDD_TO_REG(val)            clamp_val((((val) * 4 + 47) / 95), 0, 
> > 255)
> > +#define VDD_TO_REG(val) \
> > +   DIV_ROUND_CLOSEST(clamp_val(val, 0, 255 * 95 / 4) * 4, 95)
> >  #define VDD_FROM_REG(val)  (((val) * 95 + 2) / 4)
> >  
> >  #define DIV_FROM_REG(val)  (1 << (val))
> 
> Code looks good. Alignment is a bit clumsy though. Also it feels
> strange now that you are using DIV_ROUND_CLOSEST for VDD_TO_REG but not
> VDD_FROM_REG.
> 
I cleaned up the alignment, and added DIV_ROUND_CLOSEST() to VDD_TO_REG().
I'll resend an updated version.

Thanks,
Guenter

> Reviewed-by: Jean Delvare <[email protected]>
> 
> -- 
> Jean Delvare
> SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to