Wolfram Sang wrote: > On Thu, Dec 17, 2009 at 04:16:41PM +0200, Daniel Baluta wrote: >> On Thu, Dec 17, 2009 at 4:04 PM, Wolfram Sang <[email protected]> wrote: >>>> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h >>>> index 3db7767..b70d6f3 100644 >>>> --- a/include/linux/can/dev.h >>>> +++ b/include/linux/can/dev.h >>>> @@ -77,6 +77,16 @@ void can_put_echo_skb(struct sk_buff *skb, struct >>>> net_device *dev, >>>> void can_get_echo_skb(struct net_device *dev, unsigned int idx); >>>> void can_free_echo_skb(struct net_device *dev, unsigned int idx); >>>> >>>> +static inline int can_validate_skb(struct sk_buff *skb) >>>> +{ >>>> + struct can_frame *cf = (struct can_frame *)skb->data; >>>> + >>>> + if (unlikely(skb->len != sizeof(struct can_frame) || cf->can_dlc > >>>> 8)) >>>> + return 1; >>> Maybe -Esomething? >>> >>>> + >>>> + return 0; >>>> +} >>>> + >> or perhaps: >> return (unlikely(skb->len != sizeof(struct can_frame) || cf->can_dlc > 8) > > This is a new ndo_call, so it should follow common patterns IMHO. > > That would be pretty much > > Returning an int and use -E... > > or maybe > > Returning a bool, then we can do like you suggested above > > But I think it is better using an int here, you never know what other > ndo-users > may need. And ndo_validate_addr uses this scheme, too. > > Regards, > > Wolfram > > PS: Just saw that can_validate_skb() is inline. Why is that? >
For all these details please check ndo_validate_addr() one line above ... In common ethernet drivers ndo_validate_addr is set to eth_validate_addr() which is defined as extern in http://lxr.linux.no/#linux+v2.6.32/include/linux/etherdevice.h#L47 and implemented in http://lxr.linux.no/#linux+v2.6.32/net/ethernet/eth.c#L315 As net/ethernet/eth.c is always built inside the kernel, it can be referenced. Our can_validate_skb() would reside in can_dev.ko which can be a module. Therefore it's inline (which is not soo bad, as it is pretty short). Besides of this difference i'm following all the common patterns. Regards, Oliver @@ -633,6 +636,10 @@ struct net_device_ops { void *addr); #define HAVE_VALIDATE_ADDR int (*ndo_validate_addr)(struct net_device *dev); + +#define HAVE_VALIDATE_SKB + int (*ndo_validate_skb)(struct sk_buff *skb); + #define HAVE_PRIVATE_IOCTL int (*ndo_do_ioctl)(struct net_device *dev, struct ifreq *ifr, int cmd); _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
