From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933019AbcBIRmD (ORCPT ); Tue, 9 Feb 2016 12:42:03 -0500 Received: from e19.ny.us.ibm.com ([129.33.205.209]:38078 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755369AbcBIRl6 (ORCPT ); Tue, 9 Feb 2016 12:41:58 -0500 X-IBM-Helo: d01dlp02.pok.ibm.com X-IBM-MailFrom: manoj@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org;linux-scsi@vger.kernel.org Reply-To: manoj@linux.vnet.ibm.com Subject: Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values References: <1454542114-1797-1-git-send-email-tyreld@linux.vnet.ibm.com> <1454542114-1797-3-git-send-email-tyreld@linux.vnet.ibm.com> <56B3ACC9.60009@linux.vnet.ibm.com> <56B3C1C1.3080804@linux.vnet.ibm.com> To: Tyrel Datwyler , JBottomley@odin.com Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, hare@suse.de, brking@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org From: Manoj Kumar Organization: IBM Message-ID: <56BA24E1.9090808@linux.vnet.ibm.com> Date: Tue, 9 Feb 2016 11:41:53 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56B3C1C1.3080804@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16020917-0057-0000-0000-00000360DCE0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > Yeah, I can see how that is confusing. Since, all three possible valid > crq message types have the first bit set I think this was originally a > cute hack to grab anything that was likely valid. Then in > ibmvscsi_handle_crq() we explicitly match the full header value in a > switch statement logging anything that turned out actually invalid. > >> >> If 'valid' will only have one of these four enums defined, would >> this be better written as: >> >> if (crq->valid != VIOSRP_CRQ_FREE) > > This definitely would make the logic easier to read and follow. Also, > this would make sure any crq with an invalid header that doesn't have > its first bit set will also be logged by the ibmvscsi_handle_crq() > switch statement default block and not silently ignored. > > -Tyrel Sounds good, Tyrel. Does this mean I should expect a v2 of this patch series? - Manoj N. Kumar