diff mbox

[1/1] staging: cleanup: Fix incompatible type comparison in wilc1000/host_interface.c

Message ID 1448928544-3534-1-git-send-email-mrugiero@gmail.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Mario J. Rugiero Dec. 1, 2015, 12:09 a.m. UTC
This patch replaces an "if (ptr > 0)" comparison that seems to be a
confusing way to check for null by a simpler "if (ptr)" check.

Signed-off-by: Mario J. Rugiero <mrugiero@gmail.com>
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dan Carpenter Dec. 2, 2015, 1:29 p.m. UTC | #1
Put v2 in the subject.  Also the subsystem prefix is:

[PATCH v3] staging: wilc1000: ...

On Mon, Nov 30, 2015 at 09:09:04PM -0300, Mario J. Rugiero wrote:
> This patch replaces an "if (ptr > 0)" comparison that seems to be a
> confusing way to check for null by a simpler "if (ptr)" check.
> 
> Signed-off-by: Mario J. Rugiero <mrugiero@gmail.com>
> ---
v2: Remove the != NULL because checkpatch complains.

>  drivers/staging/wilc1000/host_interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
> index d5b7725..0c87f6c 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -2270,7 +2270,7 @@ static void Handle_AddBeacon(struct host_if_drv *hif_drv,
>  	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 16) & 0xFF);
>  	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 24) & 0xFF);
>  
> -	if (pstrSetBeaconParam->tail > 0)
> +	if (pstrSetBeaconParam->tail)

Probably the intention was to check if "pstrSetBeaconParam->tail_len > 0"
but I'm not sure.  The wilc1000 maintainers are very responsive though
so maybe they will know for sure.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mario J. Rugiero Dec. 2, 2015, 3:12 p.m. UTC | #2
OK. Since the maintainers are CC'd, I guess I should wait for a 
clarification about this?

El 02/12/15 a las 10:29, Dan Carpenter escribió:
> Put v2 in the subject.  Also the subsystem prefix is:
>
> [PATCH v3] staging: wilc1000: ...
>
> On Mon, Nov 30, 2015 at 09:09:04PM -0300, Mario J. Rugiero wrote:
>> This patch replaces an "if (ptr > 0)" comparison that seems to be a
>> confusing way to check for null by a simpler "if (ptr)" check.
>>
>> Signed-off-by: Mario J. Rugiero <mrugiero@gmail.com>
>> ---
> v2: Remove the != NULL because checkpatch complains.
>
>>   drivers/staging/wilc1000/host_interface.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
>> index d5b7725..0c87f6c 100644
>> --- a/drivers/staging/wilc1000/host_interface.c
>> +++ b/drivers/staging/wilc1000/host_interface.c
>> @@ -2270,7 +2270,7 @@ static void Handle_AddBeacon(struct host_if_drv *hif_drv,
>>   	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 16) & 0xFF);
>>   	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 24) & 0xFF);
>>   
>> -	if (pstrSetBeaconParam->tail > 0)
>> +	if (pstrSetBeaconParam->tail)
> Probably the intention was to check if "pstrSetBeaconParam->tail_len > 0"
> but I'm not sure.  The wilc1000 maintainers are very responsive though
> so maybe they will know for sure.
>
> regards,
> dan carpenter
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Glen Lee Dec. 3, 2015, 8:24 a.m. UTC | #3
On 2015? 12? 03? 00:12, Mario J. Rugiero wrote:
> OK. Since the maintainers are CC'd, I guess I should wait for a 
> clarification about this?

Hi Mario,

The purpose of that is to check if tail is null or not. if it is not null, there is tail
to send, so copy to buff.

regards,
glen lee.

>
> El 02/12/15 a las 10:29, Dan Carpenter escribió:
>> Put v2 in the subject.  Also the subsystem prefix is:
>>
>> [PATCH v3] staging: wilc1000: ...
>>
>> On Mon, Nov 30, 2015 at 09:09:04PM -0300, Mario J. Rugiero wrote:
>>> This patch replaces an "if (ptr > 0)" comparison that seems to be a
>>> confusing way to check for null by a simpler "if (ptr)" check.
>>>
>>> Signed-off-by: Mario J. Rugiero <mrugiero@gmail.com>
>>> ---
>> v2: Remove the != NULL because checkpatch complains.
>>
>>> drivers/staging/wilc1000/host_interface.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/wilc1000/host_interface.c 
>>> b/drivers/staging/wilc1000/host_interface.c
>>> index d5b7725..0c87f6c 100644
>>> --- a/drivers/staging/wilc1000/host_interface.c
>>> +++ b/drivers/staging/wilc1000/host_interface.c
>>> @@ -2270,7 +2270,7 @@ static void Handle_AddBeacon(struct 
>>> host_if_drv *hif_drv,
>>>       *pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 16) & 0xFF);
>>>       *pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 24) & 0xFF);
>>>   -    if (pstrSetBeaconParam->tail > 0)
>>> +    if (pstrSetBeaconParam->tail)
>> Probably the intention was to check if "pstrSetBeaconParam->tail_len 
>> > 0"
>> but I'm not sure.  The wilc1000 maintainers are very responsive though
>> so maybe they will know for sure.
>>
>> regards,
>> dan carpenter
>>
>>
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mario J. Rugiero Dec. 3, 2015, 3:13 p.m. UTC | #4
Thank you. Then the patch itself should see no changes as of now, am I 
right?
Should I submit a new one anyway to fix the subject line?

Regards,
Mario.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Dec. 3, 2015, 3:17 p.m. UTC | #5
On Thu, Dec 03, 2015 at 12:13:59PM -0300, Mario J. Rugiero wrote:
> Thank you. Then the patch itself should see no changes as of now, am
> I right?
> Should I submit a new one anyway to fix the subject line?

Yes, please.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mario J. Rugiero Dec. 3, 2015, 3:18 p.m. UTC | #6
Would be v3, or remains v2 because of the lack of further changes 
afterwards?

Regards,
Mario.

El 03/12/15 a las 12:17, Dan Carpenter escribió:
> On Thu, Dec 03, 2015 at 12:13:59PM -0300, Mario J. Rugiero wrote:
>> Thank you. Then the patch itself should see no changes as of now, am
>> I right?
>> Should I submit a new one anyway to fix the subject line?
> Yes, please.
>
> regards,
> dan carpenter
>

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Dec. 3, 2015, 3:21 p.m. UTC | #7
On Thu, Dec 03, 2015 at 12:18:34PM -0300, Mario J. Rugiero wrote:
> Would be v3, or remains v2 because of the lack of further changes
> afterwards?

Bump the version number.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/wilc1000/host_interface.c b/drivers/staging/wilc1000/host_interface.c
index d5b7725..0c87f6c 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2270,7 +2270,7 @@  static void Handle_AddBeacon(struct host_if_drv *hif_drv,
 	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 16) & 0xFF);
 	*pu8CurrByte++ = ((pstrSetBeaconParam->tail_len >> 24) & 0xFF);
 
-	if (pstrSetBeaconParam->tail > 0)
+	if (pstrSetBeaconParam->tail)
 		memcpy(pu8CurrByte, pstrSetBeaconParam->tail, pstrSetBeaconParam->tail_len);
 	pu8CurrByte += pstrSetBeaconParam->tail_len;