linux-kernel - Re: [Resend][PATCH] usb driver for intellon int51x1 based PLC like devolo dlan duo

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0904181834210.3691@wrl-59.cs.helsinki.fi>
Date:	Sat, 18 Apr 2009 18:45:29 +0300 (EEST)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Peter Holik <peter@...ik.at>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Netdev <netdev@...r.kernel.org>, linux-usb@...r.kernel.org
Subject: Re: [Resend][PATCH] usb driver for intellon int51x1 based PLC like
      devolo dlan duo

On Sat, 18 Apr 2009, Peter Holik wrote:

> >> +static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev,
> >> +		struct sk_buff *skb, gfp_t flags)
> >> +{
> >> +	int pack_len = skb->len;
> >> +	int headroom = skb_headroom(skb);
> >> +	int tailroom = skb_tailroom(skb);
> >> +	int need_tail = 0;
> >> +	__le16 *len;
> >> +
> >> +	/*
> >> +	 * usbnet would send a ZLP if packetlength mod urbsize == 0 for us,
> >> +	 * but we need to know ourself, because this would add to the length
> >> +	 * we send down to the device...
> >> +	 */
> >> +	if (!((pack_len + INT51X1_HEADER_SIZE) % dev->maxpacket))
> >> +		need_tail = 1;
> >> +
> >> +	/* if packet and our header is smaler than 64 pad to 64 (+ ZLP) */
> >> +	if ((pack_len + INT51X1_HEADER_SIZE + need_tail) < dev->maxpacket + 1)
> >> +		need_tail = dev->maxpacket + 1 - pack_len - INT51X1_HEADER_SIZE;
> >
> > This is totally crazy code fragment, first need_tail is used like a
> > boolean? But on the same some +1 scalar trick is being done? Is there some
> > reason why DIV_ROUND_UP (linux/kernel.h) won't do what you want here and
> > then you can trivally find diff = new - old ?
> >
> 
> maybe this version is not so crazy for you?
> 
>         /* if packet and our header is smaler than 64 pad to 64 (+ ZLP) */
>         if ((pack_len + INT51X1_HEADER_SIZE) < dev->maxpacket)
>                 need_tail = dev->maxpacket - pack_len - INT51X1_HEADER_SIZE + 1;
>         /*
>          * usbnet would send a ZLP if packetlength mod urbsize == 0 for us,
>          * but we need to know ourself, because this would add to the length
>          * we send down to the device...
>          */
>         else if (!((pack_len + INT51X1_HEADER_SIZE) % dev->maxpacket))
>                 need_tail = 1;

Certainly looks more obvious, having that + need_tail and the related + 1 
on the other side of the condition made the original very hard to read 
(and I failed to read it correctly which was the reason for the wrong 
DIV_ROUND_UP comment I made). This one is much better. I guess adding
a temporary var for pack_len + INT51X1_HEADER_SIZE would also make it 
somewhat nicer looking.


-- 
 i.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/