Kurt Van Dijck wrote: >>>> I still believe that the message and the backtrace does not help the >>>> user to understand that he has done something wrong. Nevertheless, if >>>> there is nobody else sharing my concerns, please go ahead preparing the >>>> patch as you suggested above. The future will show if they are reasonable. >>> I share a bit the concern, but if I understand well, this is to avoid >>> PF_PACKET sockets to break CAN rules. When regular PF_CAN is used, the >>> user will be informed via can_send in af_can.c, isn't it? >> Yes, that's also my understanding: >> >> http://lxr.linux.no/#linux+v2.6.32/net/can/af_can.c#L218 > > Wolfgang, > > To address your concern, would a construction like this suit, when > fitted in Oliver's proposal? > > inline int no_can_skb((struct sk_buff *skb) > { > struct can_frame *cf = (struct can_frame *)skb->data > > if ((skb->len != sizeof(*cf)) || ((cf->can_dlc > 8)) { > if (skb->sk && !sock_flag(skb->sk, SOCK_DEAD)) { > skb->sk->sk_err = EINVAL; > skb->sk->sk_error_report(skb->sk); /* can this block?*/ > } > > WARN_ONCE(1, "non conform skbuf: ..."); > "Dropped non conform skbuf: len %d, can_dlc %d\n", > skb->len, cf->can_dlc); > return 1; > } > return 0; > }
My primary concern is about using *WARN_ONCE*. The BUG, WARN, functions and friends indicate to the user that there is a problem with the kernel, e.g. a bug and therefore I prefer a simple dev_err(). Also the word "skbuf" does say little to the normal Linux users. I find s/skbuf/packet/ more intuitive. Of course, if there is a better way to inform the user we should use it. Unfortunately, I can't tell if your approach will work. Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
