From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38802) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QrvGa-0004SY-8Y for qemu-devel@nongnu.org; Fri, 12 Aug 2011 13:11:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QrvGZ-0005zU-Aq for qemu-devel@nongnu.org; Fri, 12 Aug 2011 13:11:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36960) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QrvGY-0005z1-WB for qemu-devel@nongnu.org; Fri, 12 Aug 2011 13:11:27 -0400 Message-ID: <4E455EBA.5000007@redhat.com> Date: Fri, 12 Aug 2011 19:11:22 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1312361359-15445-1-git-send-email-pbonzini@redhat.com> <1312361359-15445-11-git-send-email-pbonzini@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/16] scsi: move request parsing to common code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org On 08/12/2011 06:55 PM, Peter Maydell wrote: > Does it still make sense to set req->cmd to cmd (and to look at cmd > at all) in the case where scsi_req_parse() failed and might not have > actually initialised all of cmd? For instance the tracing code (added > to scsi_req_new() after this patch) looks at cmd.buf[] based on the > value of buf[0], which seems kind of fragile to me. At the point tracing is reached, we know that cmd.buf[] has been initialized. But you're right that it is at least not tidy. We know that the size of the cdb is 16 (it is always like that, and we can make it a requirement). So we can copy it to cmd->buf before knowing cmd->len, at the beginning of scsi_req_parse. We can also zero unconditionally len/xfer/mode (plus set lba to -1) in case the parsing fails. Paolo