Re: [PATCH 5/7] can: dev: add berr_limit infrastrucutre

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

On 10/07/2013 05:56 PM, Marc Kleine-Budde wrote:
> On 10/07/2013 05:39 PM, Alexander Stein wrote:
>> Hello Marc,
>>
>> On Monday 07 October 2013 16:40:38, Marc Kleine-Budde wrote:
>>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
>>> index bda1888..969d3cd 100644
>>> --- a/drivers/net/can/dev.c
>>> +++ b/drivers/net/can/dev.c
>>> @@ -486,6 +486,29 @@ void can_bus_off(struct net_device *dev)
>>>  }
>>>  EXPORT_SYMBOL_GPL(can_bus_off);
>>>  
>>> +static void can_berr_restart(unsigned long data)
>>> +{
>>> +	struct net_device *dev = (struct net_device *)data;
>>> +	struct can_priv *priv = netdev_priv(dev);
>>> +
>>> +	netdev_dbg(dev, "berr-restart\n");
>>> +	priv->do_berr_restart(dev);
>>      ^^^^^^^^^^^^^^^^^^^^^^^^
>> see below!
>>
>>> +}
>>> +
>>> +void can_berr_limit(struct net_device *dev)
>>> +{
>>> +	struct can_priv *priv = netdev_priv(dev);
>>> +
>>> +	if (priv->berr_limit_delay) {
>>> +		netdev_dbg(dev, "berr-limit\n");
>>> +		mod_timer(&priv->berr_limit_timer,
>>> +			  jiffies + priv->berr_limit_delay);
>>> +	} else {
>>> +		priv->do_berr_restart(dev);
>>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>> Here you are silently requiring that do_berr_restart is non-NULL. I
>> know that the driver has to properly set this when using
>> can_berr_limit, but it might seem confusing.
> 
> If the driver wants to make use of this feature, it has to setup this
> callback. If I add NULL pointer checks, the kernel wouldn't blow up, but
> the driver doesn't work any more.

I mean, if the driver doesn't setup the callback and there is a NULL
pointer checks in both functions, the driver doesn't work properly,
because the bus error interrupts will stay disabled and you might not
notice it, because there isn't any big sign of failure (like the NULL
pointer deref).

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]