>-----Original Message-----
>From: Palande Ameya (Nokia-D/Helsinki)
>Sent: 21 January, 2010 18:33
>To: ext Andy Shevchenko
>Cc: linux-omap@xxxxxxxxxxxxxxx; omar.ramirez@xxxxxx;
>nm@xxxxxx; deepak.chitriki@xxxxxx; Kukkonen Mika (Nokia-D/Helsinki)
>Subject: Re: [PATCH] DSPBRIDGE: Various compile warning fixes
>
>On Thu, 2010-01-21 at 17:01 +0100, ext Andy Shevchenko wrote:
>> On Thu, Jan 21, 2010 at 3:40 PM, Ameya Palande
><ameya.palande@xxxxxxxxx> wrote:
>> > --- a/drivers/dsp/bridge/wmd/io_sm.c
>> > +++ b/drivers/dsp/bridge/wmd/io_sm.c
>> > @@ -1210,7 +1210,7 @@ static void InputChnl(struct IO_MGR
>*pIOMgr, struct CHNL_OBJECT *pChnl,
>> > pChnlMgr->uWordSize;
>> > chnlId = IO_GetValue(pIOMgr->hWmdContext, struct
>SHM, sm, inputId);
>> > dwArg = IO_GetLong(pIOMgr->hWmdContext, struct SHM,
>sm, arg);
>> > - if (!(chnlId >= 0) || !(chnlId < CHNL_MAXCHANNELS)) {
>> > + if (chnlId >= CHNL_MAXCHANNELS) {
>> Could chnlld be less than zero? Anyway better to test:
>> (chnlId >= CHNL_MAXCHANNELS || chnlld < 0)
>>
>
>chnlId is defined as u32 ;)
Yeah, this is the ages old discussion about whether it makes
sense to test if unsigned is smaller than zero.
a) compiler complain (with extra flags), since it is NOP
b) from casual reader viewpoint, it is nice annotation
(you know the range of the value without needing to
check the type)
c) on the other hand, it may mislead somebody to think that
the variable is signed
d) use of unsigned should always be a design decision, and
things get pretty dangerous when there are lot of layers
(I think over the years I have at least twice seen a driver
that returned on lower levels -ERRNO that got assigned to
unsigned at higher levels)
Summary: Linus likes the check, compiler (and I) don't.
Choose your side :-)
--MiKu--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
