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

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

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

On 10/08/2013 08:03 AM, Alexander Stein wrote:
> On Monday 07 October 2013 18:00:45, Marc Kleine-Budde wrote:
>>>>> +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).
> 
> Mh, what would be the effect if a driver doesn't setup the callback
> and still uses can_berr_limit? The NULL dereference raises and OOPS
> and what then? The corresponding application gets killed? Well if the
> system as a whole still works and even better then faulty driver can
> be unloaded again, this might be suitable.

It makes no sense to use can_berr_limit() without implementing the
callback. So it makes no sense from my point of view to add a NULL
pointer check.

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 --]