From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38035) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1BzK-0002JU-QZ for qemu-devel@nongnu.org; Mon, 22 Jul 2013 05:01:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1BzI-0005i5-Mc for qemu-devel@nongnu.org; Mon, 22 Jul 2013 05:01:02 -0400 Message-ID: <51ECF49F.8070201@adacore.com> Date: Mon, 22 Jul 2013 11:00:15 +0200 From: Fabien Chouteau MIME-Version: 1.0 References: <1374254353.5357.17@snotra> In-Reply-To: <1374254353.5357.17@snotra> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Wood Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 07/19/2013 07:19 PM, Scott Wood wrote: > On 07/19/2013 04:22:46 AM, Fabien Chouteau wrote: >> On 07/18/2013 10:37 PM, Scott Wood wrote: >> > On 07/18/2013 04:27:50 AM, Fabien Chouteau wrote: >> >> The BD is full, we will have to put the rest of padding in the next one. >> > >> > What rest of padding? I thought you said rx_padding was 2 somehow? If that were true, then it would be zero at the end. >> > >> >> Read my description again. > > OK, I was confused by your answering "yes" to "wouldn't *size be 2 > here" -- I thought it was clear from the context that I was talking > about at the time of the "if (*size == etsec->rx_padding)" statement. > Maybe you thought I was talking about what would happen at that > if-statement in the next descriptor? > > In any case, it's a bit hard to follow this code. I agree it's a little bit hard, I will add some comments to make it clearer. > rx_fcb_size is included in to_write, but not in *size. FCB is not considered as part of the packet, so not in remaining_data. > rx_padding is included in *size, but not in to_write. Padding on the other side is considered as part of the packet but not allocated in the buffer. > And it generally makes me nervous to see code that will go into an > infinite loop (or other odd behavior) unless an exact-equality > terminating condition is met, especially when it's as complicated as > this, without an assertion to check for the bad case (even if it looks > like it could never happen). > For me it is more confusing to see a less-or-equal when equal is the necessary and sufficient condition. It will be even harder to understand the code... I will modify the code a bit, tell me if it's clearer. Regards, -- Fabien Chouteau