linux-kernel - Re: [PATCH 3/3] media: atomisp: Use max() macros

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZvaAvQrT1WYyXVCJ@smile.fi.intel.com>
Date: Fri, 27 Sep 2024 12:54:05 +0300
From: Andy Shevchenko <andy@...nel.org>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Benoit Parrot <bparrot@...com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Bingbu Cao <bingbu.cao@...el.com>,
	Tianshu Qiu <tian.shu.qiu@...el.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Hans Verkuil <hverkuil-cisco@...all.nl>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-staging@...ts.linux.dev
Subject: Re: [PATCH 3/3] media: atomisp: Use max() macros

On Fri, Sep 27, 2024 at 09:42:15AM +0000, Ricardo Ribalda wrote:
> The max() macro produce nicer code and also fixes the following cocci
> errors:
> 
> drivers/staging/media/atomisp/pci/sh_css_frac.h:40:17-18: WARNING opportunity for max()
> drivers/staging/media/atomisp/pci/sh_css_frac.h:50:17-18: WARNING opportunity for max()

In some (rare) cases it's a bad advice.
NAK.

...

> -	v >>= fit_shift > 0 ? fit_shift : 0;
> +	v >>= max(fit_shift, 0);

max() with 0 is a bit unusual, esp. taking into account that the operator here
is a right shift. So, what we check here is the signedness of the value to
avoid not only potentially wrong calculations, but also UB. The original code
is clearer for all this.

-- 
With Best Regards,
Andy Shevchenko