* [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter()
@ 2015-06-09 9:11 Stefan Schmidt
2015-06-09 9:21 ` Varka Bhadram
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Schmidt @ 2015-06-09 9:11 UTC (permalink / raw)
To: linux-wpan; +Cc: Alexander Aring, Varka Bhadram, Stefan Schmidt
neither ram nor register write return values have been checked here. Checking
both now. Assign ret with 0 as all other assignments are inside if blocks and
might not happen before we return ret.
CID: 1230469
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
Cc: Varka Bhadram <varkabhadram@gmail.com>
---
drivers/net/ieee802154/cc2520.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
index 15f263c..54c15e0 100644
--- a/drivers/net/ieee802154/cc2520.c
+++ b/drivers/net/ieee802154/cc2520.c
@@ -590,22 +590,23 @@ cc2520_filter(struct ieee802154_hw *hw,
struct ieee802154_hw_addr_filt *filt, unsigned long changed)
{
struct cc2520_private *priv = hw->priv;
+ int ret = 0;
if (changed & IEEE802154_AFILT_PANID_CHANGED) {
u16 panid = le16_to_cpu(filt->pan_id);
dev_vdbg(&priv->spi->dev,
"cc2520_filter called for pan id\n");
- cc2520_write_ram(priv, CC2520RAM_PANID,
- sizeof(panid), (u8 *)&panid);
+ ret = cc2520_write_ram(priv, CC2520RAM_PANID,
+ sizeof(panid), (u8 *)&panid);
}
if (changed & IEEE802154_AFILT_IEEEADDR_CHANGED) {
dev_vdbg(&priv->spi->dev,
"cc2520_filter called for IEEE addr\n");
- cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
- sizeof(filt->ieee_addr),
- (u8 *)&filt->ieee_addr);
+ ret = cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
+ sizeof(filt->ieee_addr),
+ (u8 *)&filt->ieee_addr);
}
if (changed & IEEE802154_AFILT_SADDR_CHANGED) {
@@ -613,20 +614,20 @@ cc2520_filter(struct ieee802154_hw *hw,
dev_vdbg(&priv->spi->dev,
"cc2520_filter called for saddr\n");
- cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
- sizeof(addr), (u8 *)&addr);
+ ret = cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
+ sizeof(addr), (u8 *)&addr);
}
if (changed & IEEE802154_AFILT_PANC_CHANGED) {
dev_vdbg(&priv->spi->dev,
"cc2520_filter called for panc change\n");
if (filt->pan_coord)
- cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
+ ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
else
- cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
+ ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
}
- return 0;
+ return ret;
}
static inline int cc2520_set_tx_power(struct cc2520_private *priv, s32 mbm)
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter()
2015-06-09 9:11 [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter() Stefan Schmidt
@ 2015-06-09 9:21 ` Varka Bhadram
2015-06-09 9:51 ` Stefan Schmidt
0 siblings, 1 reply; 5+ messages in thread
From: Varka Bhadram @ 2015-06-09 9:21 UTC (permalink / raw)
To: Stefan Schmidt, linux-wpan; +Cc: Alexander Aring
Hi Stefan,
On 06/09/2015 02:41 PM, Stefan Schmidt wrote:
> neither ram nor register write return values have been checked here. Checking
> both now. Assign ret with 0 as all other assignments are inside if blocks and
> might not happen before we return ret.
>
> CID: 1230469
> Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> Cc: Varka Bhadram <varkabhadram@gmail.com>
> ---
> drivers/net/ieee802154/cc2520.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
> index 15f263c..54c15e0 100644
> --- a/drivers/net/ieee802154/cc2520.c
> +++ b/drivers/net/ieee802154/cc2520.c
> @@ -590,22 +590,23 @@ cc2520_filter(struct ieee802154_hw *hw,
> struct ieee802154_hw_addr_filt *filt, unsigned long changed)
> {
> struct cc2520_private *priv = hw->priv;
> + int ret = 0;
>
> if (changed & IEEE802154_AFILT_PANID_CHANGED) {
> u16 panid = le16_to_cpu(filt->pan_id);
>
> dev_vdbg(&priv->spi->dev,
> "cc2520_filter called for pan id\n");
> - cc2520_write_ram(priv, CC2520RAM_PANID,
> - sizeof(panid), (u8 *)&panid);
> + ret = cc2520_write_ram(priv, CC2520RAM_PANID,
> + sizeof(panid), (u8 *)&panid);
> }
>
> if (changed & IEEE802154_AFILT_IEEEADDR_CHANGED) {
> dev_vdbg(&priv->spi->dev,
> "cc2520_filter called for IEEE addr\n");
> - cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
> - sizeof(filt->ieee_addr),
> - (u8 *)&filt->ieee_addr);
> + ret = cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
> + sizeof(filt->ieee_addr),
> + (u8 *)&filt->ieee_addr);
> }
>
> if (changed & IEEE802154_AFILT_SADDR_CHANGED) {
> @@ -613,20 +614,20 @@ cc2520_filter(struct ieee802154_hw *hw,
>
> dev_vdbg(&priv->spi->dev,
> "cc2520_filter called for saddr\n");
> - cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
> - sizeof(addr), (u8 *)&addr);
> + ret = cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
> + sizeof(addr), (u8 *)&addr);
> }
>
> if (changed & IEEE802154_AFILT_PANC_CHANGED) {
> dev_vdbg(&priv->spi->dev,
> "cc2520_filter called for panc change\n");
> if (filt->pan_coord)
> - cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
> + ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
> else
> - cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
> + ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
> }
>
> - return 0;
> + return ret;
> }
>
> static inline int cc2520_set_tx_power(struct cc2520_private *priv, s32 mbm)
Thanks for the patch.
I found two check patch warnings on this patch.
As Alex pointed on other thread that you should use commit message
like "subsytem: file:" or only "subsytem:" in the subject.
But not "subsytem/subsubsystem:"
Thanks.
--
Varka Bhadram
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter()
2015-06-09 9:21 ` Varka Bhadram
@ 2015-06-09 9:51 ` Stefan Schmidt
2015-06-09 10:02 ` Alexander Aring
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Schmidt @ 2015-06-09 9:51 UTC (permalink / raw)
To: Varka Bhadram, linux-wpan; +Cc: Alexander Aring
Hello.
On 09/06/15 11:21, Varka Bhadram wrote:
> Hi Stefan,
>
> On 06/09/2015 02:41 PM, Stefan Schmidt wrote:
>
>> neither ram nor register write return values have been checked here.
>> Checking
>> both now. Assign ret with 0 as all other assignments are inside if
>> blocks and
>> might not happen before we return ret.
>>
>> CID: 1230469
>> Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
>> Cc: Varka Bhadram <varkabhadram@gmail.com>
>> ---
>> drivers/net/ieee802154/cc2520.c | 21 +++++++++++----------
>> 1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ieee802154/cc2520.c
>> b/drivers/net/ieee802154/cc2520.c
>> index 15f263c..54c15e0 100644
>> --- a/drivers/net/ieee802154/cc2520.c
>> +++ b/drivers/net/ieee802154/cc2520.c
>> @@ -590,22 +590,23 @@ cc2520_filter(struct ieee802154_hw *hw,
>> struct ieee802154_hw_addr_filt *filt, unsigned long changed)
>> {
>> struct cc2520_private *priv = hw->priv;
>> + int ret = 0;
>> if (changed & IEEE802154_AFILT_PANID_CHANGED) {
>> u16 panid = le16_to_cpu(filt->pan_id);
>> dev_vdbg(&priv->spi->dev,
>> "cc2520_filter called for pan id\n");
>> - cc2520_write_ram(priv, CC2520RAM_PANID,
>> - sizeof(panid), (u8 *)&panid);
>> + ret = cc2520_write_ram(priv, CC2520RAM_PANID,
>> + sizeof(panid), (u8 *)&panid);
>> }
>> if (changed & IEEE802154_AFILT_IEEEADDR_CHANGED) {
>> dev_vdbg(&priv->spi->dev,
>> "cc2520_filter called for IEEE addr\n");
>> - cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
>> - sizeof(filt->ieee_addr),
>> - (u8 *)&filt->ieee_addr);
>> + ret = cc2520_write_ram(priv, CC2520RAM_IEEEADDR,
>> + sizeof(filt->ieee_addr),
>> + (u8 *)&filt->ieee_addr);
>> }
>> if (changed & IEEE802154_AFILT_SADDR_CHANGED) {
>> @@ -613,20 +614,20 @@ cc2520_filter(struct ieee802154_hw *hw,
>> dev_vdbg(&priv->spi->dev,
>> "cc2520_filter called for saddr\n");
>> - cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
>> - sizeof(addr), (u8 *)&addr);
>> + ret = cc2520_write_ram(priv, CC2520RAM_SHORTADDR,
>> + sizeof(addr), (u8 *)&addr);
>> }
>> if (changed & IEEE802154_AFILT_PANC_CHANGED) {
>> dev_vdbg(&priv->spi->dev,
>> "cc2520_filter called for panc change\n");
>> if (filt->pan_coord)
>> - cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
>> + ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x02);
>> else
>> - cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
>> + ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
>> }
>> - return 0;
>> + return ret;
>> }
>> static inline int cc2520_set_tx_power(struct cc2520_private
>> *priv, s32 mbm)
>
> Thanks for the patch.
>
> I found two check patch warnings on this patch.
>
Both fixed.
> As Alex pointed on other thread that you should use commit message
> like "subsytem: file:" or only "subsytem:" in the subject.
> But not "subsytem/subsubsystem:"
>
Well, imho its not a subsubsystem but rather a simple file. Also using
subsytem: file: looks quite alien to me. Anyway, matter of taste,
nothing to argue about. Fixed in v2.
regards
Stefan Schmidt
> Thanks.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter()
2015-06-09 9:51 ` Stefan Schmidt
@ 2015-06-09 10:02 ` Alexander Aring
2015-06-09 11:50 ` Stefan Schmidt
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Aring @ 2015-06-09 10:02 UTC (permalink / raw)
To: Stefan Schmidt; +Cc: Varka Bhadram, linux-wpan
On Tue, Jun 09, 2015 at 11:51:27AM +0200, Stefan Schmidt wrote:
>
> Well, imho its not a subsubsystem but rather a simple file. Also using
> subsytem: file: looks quite alien to me. Anyway, matter of taste, nothing to
> argue about. Fixed in v2.
>
sorry, I never see foo/bar: things at begin of the subject. I see David
Miller on netdev which only mentioned that the usually way is
"subsystem:", when somebody complete forgets to add this. For me it doesn't matter
if somebody will more specify it like "subsystem: drivers: file:" or
"subsystem: file: ..." that's everything fine, but don making a '/' somewhere.
But I see also people which tags "6lowpan:" only and then it touched
net/ieee802154/6lowpan only. That's confusing, it should be then
"ieee802154: 6lowpan:". If it touched the net/6lowpan it should be
"6lowpan:" only.
I think I need to make some harder rules to check on this.
- Alex
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter()
2015-06-09 10:02 ` Alexander Aring
@ 2015-06-09 11:50 ` Stefan Schmidt
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Schmidt @ 2015-06-09 11:50 UTC (permalink / raw)
To: Alexander Aring; +Cc: Varka Bhadram, linux-wpan
Hello.
On 09/06/15 12:02, Alexander Aring wrote:
> On Tue, Jun 09, 2015 at 11:51:27AM +0200, Stefan Schmidt wrote:
>> Well, imho its not a subsubsystem but rather a simple file. Also using
>> subsytem: file: looks quite alien to me. Anyway, matter of taste, nothing to
>> argue about. Fixed in v2.
>>
> sorry, I never see foo/bar: things at begin of the subject. I see David
> Miller on netdev which only mentioned that the usually way is
> "subsystem:", when somebody complete forgets to add this. For me it doesn't matter
> if somebody will more specify it like "subsystem: drivers: file:" or
> "subsystem: file: ..." that's everything fine, but don making a '/' somewhere.
I see both forms used regularly when looking through git log. Anyway, if
the preferred form is subsystem: drivers: I'm ok with that. Not making
any fuzz about it. Maybe I'm just used to the other notation. :)
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-09 11:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-09 9:11 [PATCH bluetooth-next] ieee802154/cc2520: check for return values in cc2520_filter() Stefan Schmidt
2015-06-09 9:21 ` Varka Bhadram
2015-06-09 9:51 ` Stefan Schmidt
2015-06-09 10:02 ` Alexander Aring
2015-06-09 11:50 ` Stefan Schmidt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.