[PATCH] smiapp: Fix BUG_ON() on an impossible condition

[PATCH] smiapp: Fix BUG_ON() on an impossible condition

[Ricardo Ribalda Delgado]

internal_csi_format_idx and csi_format_idx are unsigned integers,
therefore they can never be nevative.

CC      drivers/media/i2c/smiapp/smiapp-core.o
In file included from include/linux/err.h:4:0,
                 from include/linux/clk.h:15,
                 from drivers/media/i2c/smiapp/smiapp-core.c:29:
drivers/media/i2c/smiapp/smiapp-core.c: In function ‘smiapp_update_mbus_formats’:
include/linux/kernel.h:669:20: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
 #define min(x, y) ({    \
                    ^
include/linux/compiler.h:153:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
                                          ^
drivers/media/i2c/smiapp/smiapp-core.c:402:2: note: in expansion of macro ‘BUG_ON’
  BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
  ^
drivers/media/i2c/smiapp/smiapp-core.c:402:9: note: in expansion of macro ‘min’
  BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
         ^

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index ae66d91..fbd48f0 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -399,7 +399,6 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 
 	BUG_ON(max(internal_csi_format_idx, csi_format_idx) + pixel_order
 	       >= ARRAY_SIZE(smiapp_csi_data_formats));
-	BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
 
 	dev_dbg(&client->dev, "new pixel order %s\n",
 		pixel_order_str[pixel_order]);
-- 
1.8.4.rc3

Re: [PATCH] smiapp: Fix BUG_ON() on an impossible condition

[Sakari Ailus]

Hi Ricardo,

Thanks for the patch.

I've removed LKML from cc since I don't think this is anything but noise
there.

On Wed, Nov 06, 2013 at 03:21:30PM +0100, Ricardo Ribalda Delgado wrote:
> internal_csi_format_idx and csi_format_idx are unsigned integers,
> therefore they can never be nevative.
> 
> CC      drivers/media/i2c/smiapp/smiapp-core.o
> In file included from include/linux/err.h:4:0,
>                  from include/linux/clk.h:15,
>                  from drivers/media/i2c/smiapp/smiapp-core.c:29:
> drivers/media/i2c/smiapp/smiapp-core.c: In function ‘smiapp_update_mbus_formats’:
> include/linux/kernel.h:669:20: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>  #define min(x, y) ({    \
>                     ^
> include/linux/compiler.h:153:42: note: in definition of macro ‘unlikely’
>  # define unlikely(x) __builtin_expect(!!(x), 0)
>                                           ^
> drivers/media/i2c/smiapp/smiapp-core.c:402:2: note: in expansion of macro ‘BUG_ON’
>   BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>   ^
> drivers/media/i2c/smiapp/smiapp-core.c:402:9: note: in expansion of macro ‘min’
>   BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>          ^
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index ae66d91..fbd48f0 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -399,7 +399,6 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
>  
>  	BUG_ON(max(internal_csi_format_idx, csi_format_idx) + pixel_order
>  	       >= ARRAY_SIZE(smiapp_csi_data_formats));
> -	BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>  
>  	dev_dbg(&client->dev, "new pixel order %s\n",
>  		pixel_order_str[pixel_order]);

I wonder how this hasn't been noticed before. :-) No harm done, though.

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

Should I take the patch to my tree? I don't think I have other pending
patches for smiapp so I'm fine that you have it in yours, too.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] smiapp: Fix BUG_ON() on an impossible condition

[Ricardo Ribalda Delgado]

Hello Sakari

I always try to send the patches to the mails found by
get-maintainer.pl, I thought it was unpolite not doint so :). In the
future I will try to send the mails only to the media list.

I don't have a tree, so please do as you wish.


Thanks!

On Wed, Nov 6, 2013 at 3:58 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Ricardo,
>
> Thanks for the patch.
>
> I've removed LKML from cc since I don't think this is anything but noise
> there.
>
> On Wed, Nov 06, 2013 at 03:21:30PM +0100, Ricardo Ribalda Delgado wrote:
>> internal_csi_format_idx and csi_format_idx are unsigned integers,
>> therefore they can never be nevative.
>>
>> CC      drivers/media/i2c/smiapp/smiapp-core.o
>> In file included from include/linux/err.h:4:0,
>>                  from include/linux/clk.h:15,
>>                  from drivers/media/i2c/smiapp/smiapp-core.c:29:
>> drivers/media/i2c/smiapp/smiapp-core.c: In function ‘smiapp_update_mbus_formats’:
>> include/linux/kernel.h:669:20: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>  #define min(x, y) ({    \
>>                     ^
>> include/linux/compiler.h:153:42: note: in definition of macro ‘unlikely’
>>  # define unlikely(x) __builtin_expect(!!(x), 0)
>>                                           ^
>> drivers/media/i2c/smiapp/smiapp-core.c:402:2: note: in expansion of macro ‘BUG_ON’
>>   BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>>   ^
>> drivers/media/i2c/smiapp/smiapp-core.c:402:9: note: in expansion of macro ‘min’
>>   BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>>          ^
>>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  drivers/media/i2c/smiapp/smiapp-core.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
>> index ae66d91..fbd48f0 100644
>> --- a/drivers/media/i2c/smiapp/smiapp-core.c
>> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
>> @@ -399,7 +399,6 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
>>
>>       BUG_ON(max(internal_csi_format_idx, csi_format_idx) + pixel_order
>>              >= ARRAY_SIZE(smiapp_csi_data_formats));
>> -     BUG_ON(min(internal_csi_format_idx, csi_format_idx) < 0);
>>
>>       dev_dbg(&client->dev, "new pixel order %s\n",
>>               pixel_order_str[pixel_order]);
>
> I wonder how this hasn't been noticed before. :-) No harm done, though.
>
> Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
>
> Should I take the patch to my tree? I don't think I have other pending
> patches for smiapp so I'm fine that you have it in yours, too.
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi     XMPP: sailus@retiisi.org.uk



-- 
Ricardo Ribalda

Re: [PATCH] smiapp: Fix BUG_ON() on an impossible condition

[Sakari Ailus]

Hi Ricardo,

On Wed, Nov 06, 2013 at 04:03:37PM +0100, Ricardo Ribalda Delgado wrote:
> Hello Sakari
> 
> I always try to send the patches to the mails found by
> get-maintainer.pl, I thought it was unpolite not doint so :). In the
> future I will try to send the mails only to the media list.

Yeah, it seems get_maintainer.pl -f ... gives you LKML, and trying out a few
random files, it seems to give that for all of them. I don't think it's
intended that every single patch goes to LKML however.

Here's what Documentation/SubmittingPatches has to say on the topic:

------8<------
Look through the MAINTAINERS file and the source code, and determine
if your change applies to a specific subsystem of the kernel, with
an assigned maintainer.  If so, e-mail that person.  The script
scripts/get_maintainer.pl can be very useful at this step.

If no maintainer is listed, or the maintainer does not respond, send
your patch to the primary Linux kernel developer's mailing list,
linux-kernel@vger.kernel.org.  Most kernel developers monitor this
e-mail list, and can comment on your changes.


Do not send more than 15 patches at once to the vger mailing lists!!!
------8<------

So, patches should be sent to LKML if no other relevant list can be found.
But then, it goes on to state that:

------8<------
Linus Torvalds is the final arbiter of all changes accepted into the
Linux kernel.  His e-mail address is <torvalds@linux-foundation.org>.
He gets a lot of e-mail, so typically you should do your best to -avoid-
sending him e-mail.

Patches which are bug fixes, are "obvious" changes, or similarly
require little discussion should be sent or CC'd to Linus.  Patches
which require discussion or do not have a clear advantage should
usually be sent first to linux-kernel.  Only after the patch is
discussed should the patch then be submitted to Linus.
------8<------

Could this be one of the reason Linus gets lots of mail? :-)

> I don't have a tree, so please do as you wish.

I'll apply that to my tree.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk