From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [Patch 1/2] tabled: fix bugs in streaming of data Date: Tue, 05 Jan 2010 04:13:36 -0500 Message-ID: <4B4302C0.9080007@garzik.org> References: <20100105002749.4e13eb50@redhat.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=vPsbFFxST5zHVYVd/2o7aUcKTaFrOcuY46X8F4QbZOM=; b=Jwz72nC5dLvR8FD/NxNGI3KzPAdVOMBnKR4fQelmgPpOMWvcryGGhcVnkxpdCyU4eo T7lRaU029KrWp/ArI/I9K1qvJ527hBkee1L7u7UNg7iGWZ8l4q00L2b3L2szLAYOk5dg mB5w1YBki2Bor91Lw1ZBlTldh1KCIS/JkTVZM= In-Reply-To: <20100105002749.4e13eb50@redhat.com> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Pete Zaitcev Cc: Project Hail List On 01/05/2010 02:27 AM, Pete Zaitcev wrote: > This patch fixes 3 plain coding bugs in the streaming mechanism. > > 1. Whenever we loop back to "restart" label, we have to clear n_iov. > Otherwise, the loop that fills iov[] writes beyond the end of the > array. The result is binary garbage on console and a hang. > > Behold the evils of initialized variables. Fix is obvious. > > BTW, array indexes are signed. > > 2. When we consume a queue element partially, we ought to advance its > buffer pointer when we decrement the length. Otherwise, we resend > the data that was sent already, causing a corruption in transfer. > > The fix is obvious (add tmp->buf += sz). > > 3. The accounting (through unfortunately named write_cnt) was incorrect > because wr->len may be decremented by the time the element is freed. > > Since we're at it, we implement a couple of safety improvements. > First, we change the API to the callback a little. From now on, it > does not have access to the struct client_write anymore, and cannot > accidentially rely on wr->buf (which can be changed now). > Also, we renamed "len" into "togo" and named the new member "length", > to make sure that all use instances for "len" were reviewed. > > Signed-Off-By: Pete Zaitcev > > --- > server/object.c | 8 +++----- > server/server.c | 31 ++++++++++++++++--------------- > server/tabled.h | 11 ++++++----- > 3 files changed, 25 insertions(+), 25 deletions(-) applied... chunkd needs these fixes also, yes?