From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754486AbbLKK0T (ORCPT ); Fri, 11 Dec 2015 05:26:19 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:17007 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751071AbbLKK0R (ORCPT ); Fri, 11 Dec 2015 05:26:17 -0500 Date: Fri, 11 Dec 2015 13:25:19 +0300 From: Dan Carpenter To: "K. Y. Srinivasan" Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, ohering@suse.com, jbottomley@parallels.com, hch@infradead.org, linux-scsi@vger.kernel.org, apw@canonical.com, vkuznets@redhat.com, jasowang@redhat.com, martin.petersen@oracle.com Subject: Re: [PATCH 2/4] scsi: storvsc: Properly support Fibre Channel devices Message-ID: <20151211102519.GF5284@mwanda> References: <1449792829-15406-1-git-send-email-kys@microsoft.com> <1449792860-15447-1-git-send-email-kys@microsoft.com> <1449792860-15447-2-git-send-email-kys@microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449792860-15447-2-git-send-email-kys@microsoft.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 10, 2015 at 04:14:18PM -0800, K. Y. Srinivasan wrote: > + ret = vmbus_sendpacket(device->channel, vstor_packet, > + (sizeof(struct vstor_packet) - > + vmscsi_size_delta), > + (unsigned long)request, > + VM_PKT_DATA_INBAND, > + VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); > + > + if (ret != 0) > + goto cleanup; > + > + t = wait_for_completion_timeout(&request->wait_event, 5*HZ); > + if (t == 0) { > + ret = -ETIMEDOUT; > + goto cleanup; > + } > + > + if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO || > + vstor_packet->status != 0) > + goto cleanup; "cleanup" is a misleading name because it doesn't clean up anything. Do nothing gotos are a pain in the butt and they always introduce bugs. For example, you appear to have forgotten to set the error code. But because it's a do-nothing goto it's ambiguous so perhaps returning success was intended. Empirically this style of coding causes bugs. It does not prevent them. It is a bad style if you believe in measuring, evidence and science. regards, dan carpenter