From patchwork Mon Jan 11 20:18:02 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Bolldorf?= X-Patchwork-Id: 42660 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 7D1A7B7C36 for ; Tue, 12 Jan 2010 07:18:50 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754186Ab0AKURj (ORCPT ); Mon, 11 Jan 2010 15:17:39 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754016Ab0AKURh (ORCPT ); Mon, 11 Jan 2010 15:17:37 -0500 Received: from mail-fx0-f225.google.com ([209.85.220.225]:62948 "EHLO mail-fx0-f225.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753989Ab0AKURf (ORCPT ); Mon, 11 Jan 2010 15:17:35 -0500 Received: by fxm25 with SMTP id 25so1341fxm.21 for ; Mon, 11 Jan 2010 12:17:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=WgkmzZNTpVhsnSLVFZQa2IRjPj5vsmqGab0xHvRl+A4=; b=X+hn4AHrurjlKfMkO3v01ETmG+HOu/Os2GccbSsZF8CIz4mKvAtQCLHJUg/gSMtPFK 1UDS1p0zMiqs5cEaDq3k7zora5/7zRZ/E28F/U7VgCFt1biBHt2P/snF6AnYD13Nuwbz asl1tquf8TWVWJNGQjiAB2OuFhCw9QyUQdUp4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=Z5fjfKLZ1D3kwVg9g2GefMpHwwtdwgy7ZrMEWExbHnsxJIgsP3iBlADJRykcN3Al9w 9drMb6f40M6u0/cgBfaV+iVy/hzkZgtk4HEVFPDKtQkkGumTC8CLxInU8RdhWN7bdn0i cAtrrkDHdvOdT/gaEOjSA4XMuZ20yrFnpHlE8= Received: by 10.87.29.3 with SMTP id g3mr14006155fgj.56.1263241052724; Mon, 11 Jan 2010 12:17:32 -0800 (PST) Received: from ?192.168.1.2? (dslb-088-071-143-078.pools.arcor-ip.net [88.71.143.78]) by mx.google.com with ESMTPS id 3sm7795695fge.5.2010.01.11.12.17.31 (version=SSLv3 cipher=RC4-MD5); Mon, 11 Jan 2010 12:17:32 -0800 (PST) Message-ID: <4B4B877A.8060106@googlemail.com> Date: Mon, 11 Jan 2010 21:18:02 +0100 From: =?ISO-8859-1?Q?Ren=E9_Bolldorf?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.5) Gecko/20091204 Thunderbird/3.0 MIME-Version: 1.0 To: Marc Bejarano CC: linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org, James Bottomley , linux-kernel@vger.kernel.org, jgarzik@pobox.com Subject: Re: [PATCH]drivers/ata/libata-eh.c:1509 unneeded memset() References: <5543f88f1001111129u362be554kd97027d977b5dff3@mail.gmail.com> <5543f88f1001111141r5375d2a3kd726d2b70e124b94@mail.gmail.com> In-Reply-To: <5543f88f1001111141r5375d2a3kd726d2b70e124b94@mail.gmail.com> Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org On 01/11/10 20:41, Marc Bejarano wrote: > and once more in plain text. (sorry vger) > > 2010/1/11 Marc Bejarano: >> oops. seems that GMANE's reply function only goes to a single "newsgroup". >> original recipients re-added. >> marc >> ---- >> From: Marc Bejarano beej.org> >> Subject: Re: [PATCH]drivers/ata/libata-eh.c:1509 unneeded memset() >> Newsgroups: gmane.linux.scsi >> Date: 2010-01-08 22:21:09 GMT (2 days, 20 hours and 40 minutes ago) >> >> René Bolldorf googlemail.com> writes: >>> On 01/08/10 16:28, James Bottomley wrote: >>>> On Thu, 2010-01-07 at 14:15 -0500, Jeff Garzik wrote: >>>>> On 12/30/2009 05:59 PM, René Bolldorf wrote: >>>>>> We don't need this . >> >>>>>> - /* FIXME: is this needed? */ >>>>>> - memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE); >>>>> >>>>> I need a little bit more detail than an unqualified statement... Did >>>>> you audit all paths leading to this code point? >> >>>> But one also here: >>>> >>>> u8 *sense_buffer = dev->link->ap->sector_buf; >>>> [...] >>>> err_mask = atapi_eh_request_sense(dev, sense_buffer, sense_key); >>>> >>>> Which doesn't look OK because it looks like the sector_buf isn't cleared >>>> (and it is reused). >>>> >>>> James >>>> >>>> >>> >>> Thank's, you're right. I have overseen this, sry for that. >> >> René: perhaps you'd like to submit a patch that substitutes FIXME comment >> for one that explains why the memset is needed, crediting James in the >> description? we may as well gain something permanent from this discussion >> that you started :) >> >> cheers, >> marc >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >> the body of a message to majordomo vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> ---- I hope that's good enough explained :-). Best regards René /* initialize sense_buf with the error register, --- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- ./drivers/ata/libata-eh.c 2010-01-07 23:21:23.622464012 +0100 +++ ./drivers/ata/libata-eh.c 2010-01-11 21:11:19.869092897 +0100 @@ -1505,7 +1505,10 @@ static unsigned int atapi_eh_request_sen DPRINTK("ATAPI request sense\n"); - /* FIXME: is this needed? */ + /* make sure sense_buf is cleared then atapi_eh_request_sense is called. + * (to make sure nothing get's reused.) + * thanks to James Bottomley + */ memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);