On Mon, 2006-02-27 at 07:23 +0100, Hansjoerg Lipp wrote:
> + struct semaphore sem; /* locks this structure:
> + * connected is not changed,
> + * hardware_up is not changed,
> + * MState is not changed to or from
> + * MS_LOCKED */
> +
please turn this into a mutex
> +/* handling routines for sk_buff */
> +/* ============================= */
> +
> +/* private version of __skb_put()
> + * append 'len' bytes to the content of 'skb', already knowing that the
> + * existing buffer can accomodate them
> + * returns a pointer to the location where the new bytes should be copied to
> + * This function does not take any locks so it must be called with the
> + * appropriate locks held only.
> + */
> +static inline unsigned char *gigaset_skb_put_quick(struct sk_buff *skb,
> + unsigned int len)
> +{
> + unsigned char *tmp = skb->tail;
> + /*SKB_LINEAR_ASSERT(skb);*/ /* not needed here */
> + skb->tail += len;
> + skb->len += len;
> + return tmp;
> +}
this looks truely scary and wrong
> +/* append received bytes to inbuf */
> +static inline int gigaset_fill_inbuf(struct inbuf_t *inbuf,
> + const unsigned char *src,
> + unsigned numbytes)
> +{
> + unsigned n, head, tail, bytesleft;
> +
> + gig_dbg(DEBUG_INTR, "received %u bytes", numbytes);
> +
> + if (!numbytes)
[snip 30 lines]
isn't this function rather big to be inlined ?
> +
> +/*======================================================================
> + Prototypes of internal functions
> + */
> +
> +static struct cardstate *alloc_cs(struct gigaset_driver *drv);
> +static void free_cs(struct cardstate *cs);
> +static void make_valid(struct cardstate *cs, unsigned mask);
> +static void make_invalid(struct cardstate *cs, unsigned mask);
most of the time these can just go away by ordering the functions better
> +
> +void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> + size_t len, const unsigned char *buf, int from_user)
such "from_user" parameter is highly evil, and also breaks sparse and
friends.. (btw please run sparse on the code and fix all warnings)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
