Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753422AbbGAJgD (ORCPT ); Wed, 1 Jul 2015 05:36:03 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:18084 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751581AbbGAJfy (ORCPT ); Wed, 1 Jul 2015 05:35:54 -0400 X-IronPort-AV: E=Sophos;i="5.15,384,1432591200"; d="scan'208";a="168258352" Date: Wed, 1 Jul 2015 11:35:46 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@hadrien To: Sohny Thomas cc: Julia Lawall , Sudip Mukherjee , benjamin.romer@unisys.com, david.kershner@unisys.com, bryan.thompson@unisys.com, erik.arfvidson@unisys.com, devel@driverdev.osuosl.org, gregkh@linuxfoundation.org, sparmaintainer@unisys.com, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue In-Reply-To: <5593B0EA.1070101@zoho.com> Message-ID: References: <55930BB1.10502@zoho.com> <20150701065749.GA2411@sudip-PC> <55939890.6090903@zoho.com> <5593B0EA.1070101@zoho.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1509 Lines: 49 On Wed, 1 Jul 2015, Sohny Thomas wrote: > >>>> i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE, > >>>> &scsi.wwnn, NULL); > >>>> - if (i) { > >>>> + if (i) > >>>> return 1; > >>>> - } > >>>> - return 0; > >>>> + else > >>>> + return 0; > >>> No, now this will introduce a new checkpatch warning that "else is not > >>> required after return". why did you introduce this "else"? > >> I did this so that the code is more readable and understandable, I checked and > >> checkpatch didn't call this out , so its clean. > >> > >> Otherwise the above code looks like this > >> > >> if(i) > >> return 1; > >> return 0; > > > > That looks fine. > > > > I haven't looked at the code in detail. Is it normal that the return > > values seem to be 0 1 and -1? Which values represent success and which > > represent an error? It is nicer to have the errors under if and success > > as a direct return at the end. > Here in this driver directory, return 1 means SUCCESS and return 0 means FAILURE What is -1? > So you mean my code change is fine? I think it would be best to have the success case that is not under an if. So if (!i) return 0; return 1; I guess some day the driver would need more normal error codes? julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/