* [PATCH net-next] 8139cp: remove a won't occurred BUG_ON
@ 2014-01-26 8:33 Wang Weidong
2014-01-26 23:23 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: Wang Weidong @ 2014-01-26 8:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev
when variable i go to the BUG_ON the value is equal to the CP_NUM_STATS,
so the BUG_ON won't occur, so remove it
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
drivers/net/ethernet/realtek/8139cp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 737c1a8..b70e184 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -1585,7 +1585,6 @@ static void cp_get_ethtool_stats (struct net_device *dev,
tmp_stats[i++] = le16_to_cpu(nic_stats->tx_abort);
tmp_stats[i++] = le16_to_cpu(nic_stats->tx_underrun);
tmp_stats[i++] = cp->cp_stats.rx_frags;
- BUG_ON(i != CP_NUM_STATS);
dma_free_coherent(&cp->pdev->dev, sizeof(*nic_stats), nic_stats, dma);
}
--
1.7.12
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] 8139cp: remove a won't occurred BUG_ON
2014-01-26 8:33 [PATCH net-next] 8139cp: remove a won't occurred BUG_ON Wang Weidong
@ 2014-01-26 23:23 ` Ben Hutchings
2014-01-27 1:14 ` Wang Weidong
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2014-01-26 23:23 UTC (permalink / raw)
To: Wang Weidong; +Cc: David Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]
On Sun, 2014-01-26 at 16:33 +0800, Wang Weidong wrote:
> when variable i go to the BUG_ON the value is equal to the CP_NUM_STATS,
> so the BUG_ON won't occur, so remove it
We hope that every BUG_ON() does not occur, but that doesn't mean they
should be removed. This check is meant to catch mistakes when adding
new statistics.
Ben.
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> drivers/net/ethernet/realtek/8139cp.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> index 737c1a8..b70e184 100644
> --- a/drivers/net/ethernet/realtek/8139cp.c
> +++ b/drivers/net/ethernet/realtek/8139cp.c
> @@ -1585,7 +1585,6 @@ static void cp_get_ethtool_stats (struct net_device *dev,
> tmp_stats[i++] = le16_to_cpu(nic_stats->tx_abort);
> tmp_stats[i++] = le16_to_cpu(nic_stats->tx_underrun);
> tmp_stats[i++] = cp->cp_stats.rx_frags;
> - BUG_ON(i != CP_NUM_STATS);
>
> dma_free_coherent(&cp->pdev->dev, sizeof(*nic_stats), nic_stats, dma);
> }
--
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] 8139cp: remove a won't occurred BUG_ON
2014-01-26 23:23 ` Ben Hutchings
@ 2014-01-27 1:14 ` Wang Weidong
2014-01-27 11:54 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: Wang Weidong @ 2014-01-27 1:14 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev
On 2014/1/27 7:23, Ben Hutchings wrote:
> On Sun, 2014-01-26 at 16:33 +0800, Wang Weidong wrote:
>> when variable i go to the BUG_ON the value is equal to the CP_NUM_STATS,
>> so the BUG_ON won't occur, so remove it
>
> We hope that every BUG_ON() does not occur, but that doesn't mean they
> should be removed. This check is meant to catch mistakes when adding
> new statistics.
>
> Ben.
>
Hi, Ben.
Yeah, but I think If someone would add new statistics, he should take into account
it instead the BUG_ON helper.
And that, I found some other drivers' get_ethtool_stats no have BUG_ON. Should we
add the BUG_ON into them?
Regards,
Wang
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>> drivers/net/ethernet/realtek/8139cp.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
>> index 737c1a8..b70e184 100644
>> --- a/drivers/net/ethernet/realtek/8139cp.c
>> +++ b/drivers/net/ethernet/realtek/8139cp.c
>> @@ -1585,7 +1585,6 @@ static void cp_get_ethtool_stats (struct net_device *dev,
>> tmp_stats[i++] = le16_to_cpu(nic_stats->tx_abort);
>> tmp_stats[i++] = le16_to_cpu(nic_stats->tx_underrun);
>> tmp_stats[i++] = cp->cp_stats.rx_frags;
>> - BUG_ON(i != CP_NUM_STATS);
>>
>> dma_free_coherent(&cp->pdev->dev, sizeof(*nic_stats), nic_stats, dma);
>> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] 8139cp: remove a won't occurred BUG_ON
2014-01-27 1:14 ` Wang Weidong
@ 2014-01-27 11:54 ` Ben Hutchings
2014-01-27 12:57 ` Wang Weidong
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2014-01-27 11:54 UTC (permalink / raw)
To: Wang Weidong; +Cc: David Miller, netdev
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
On Mon, 2014-01-27 at 09:14 +0800, Wang Weidong wrote:
> On 2014/1/27 7:23, Ben Hutchings wrote:
> > On Sun, 2014-01-26 at 16:33 +0800, Wang Weidong wrote:
> >> when variable i go to the BUG_ON the value is equal to the CP_NUM_STATS,
> >> so the BUG_ON won't occur, so remove it
> >
> > We hope that every BUG_ON() does not occur, but that doesn't mean they
> > should be removed. This check is meant to catch mistakes when adding
> > new statistics.
> >
> > Ben.
> >
> Hi, Ben.
>
> Yeah, but I think If someone would add new statistics, he should take into account
> it instead the BUG_ON helper.
>
> And that, I found some other drivers' get_ethtool_stats no have BUG_ON. Should we
> add the BUG_ON into them?
[...]
The important thing is that the get_stats, get_sset_count and
get_strings operations are consistent. Depending on how they are
implemented, a BUG_ON or BUILD_BUG_ON may be useful to check that. I
don't think there's any universal best practice.
Ben.
--
Ben Hutchings
If at first you don't succeed, you're doing about average.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] 8139cp: remove a won't occurred BUG_ON
2014-01-27 11:54 ` Ben Hutchings
@ 2014-01-27 12:57 ` Wang Weidong
0 siblings, 0 replies; 5+ messages in thread
From: Wang Weidong @ 2014-01-27 12:57 UTC (permalink / raw)
To: Ben Hutchings, Wang Weidong; +Cc: David Miller, netdev
From: Wang Weidong <wangweidong1@huawei.com>
On 2014/1/27 19:54, Ben Hutchings wrote:
> On Mon, 2014-01-27 at 09:14 +0800, Wang Weidong wrote:
>> On 2014/1/27 7:23, Ben Hutchings wrote:
>>> On Sun, 2014-01-26 at 16:33 +0800, Wang Weidong wrote:
>>>> when variable i go to the BUG_ON the value is equal to the CP_NUM_STATS,
>>>> so the BUG_ON won't occur, so remove it
>>>
>>> We hope that every BUG_ON() does not occur, but that doesn't mean they
>>> should be removed. This check is meant to catch mistakes when adding
>>> new statistics.
>>>
>>> Ben.
>>>
>> Hi, Ben.
>>
>> Yeah, but I think If someone would add new statistics, he should take into account
>> it instead the BUG_ON helper.
>>
>> And that, I found some other drivers' get_ethtool_stats no have BUG_ON. Should we
>> add the BUG_ON into them?
> [...]
>
> The important thing is that the get_stats, get_sset_count and
> get_strings operations are consistent. Depending on how they are
> implemented, a BUG_ON or BUILD_BUG_ON may be useful to check that. I
> don't think there's any universal best practice.
>
> Ben.
>
Ok, Got it.
Thanks for your answers.
Regards,
Wang
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-27 12:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-26 8:33 [PATCH net-next] 8139cp: remove a won't occurred BUG_ON Wang Weidong
2014-01-26 23:23 ` Ben Hutchings
2014-01-27 1:14 ` Wang Weidong
2014-01-27 11:54 ` Ben Hutchings
2014-01-27 12:57 ` Wang Weidong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).