From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754696AbbHJRF1 (ORCPT ); Mon, 10 Aug 2015 13:05:27 -0400 Received: from mga11.intel.com ([192.55.52.93]:64653 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752679AbbHJRF0 (ORCPT ); Mon, 10 Aug 2015 13:05:26 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,646,1432623600"; d="scan'208";a="745786964" Message-ID: <1439226322.29746.227.camel@linux.intel.com> Subject: Re: [PATCH v1 3/3] device property: attach 'else if' to the proper 'if' From: Andy Shevchenko To: "Rafael J . Wysocki" , Greg Kroah-Hartman , linux-kernel@vger.kernel.org Date: Mon, 10 Aug 2015 20:05:22 +0300 In-Reply-To: <1439225808-132916-3-git-send-email-andriy.shevchenko@linux.intel.com> References: <1439225808-132916-1-git-send-email-andriy.shevchenko@linux.intel.com> <1439225808-132916-3-git-send-email-andriy.shevchenko@linux.intel.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2015-08-10 at 19:56 +0300, Andy Shevchenko wrote: > Obviously in the current place the 'else' keyword is redundant, > though it seems > quite correct when we check if nval is in allowed range. > > Reattach the condition branch there. Rafael, it would be nice to have these fixes in 4.3 as well if you have no objections. One comment below. > > Signed-off-by: Andy Shevchenko > --- > drivers/acpi/property.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 7836e2e..a28752c 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -528,13 +528,14 @@ int acpi_dev_prop_read(struct acpi_device > *adev, const char *propname, > > if (!val) > return obj->package.count; > - else if (nval <= 0) > - return -EINVAL; > > if (nval > obj->package.count) > return -EOVERFLOW; > + else if (nval <= 0) > + return -EINVAL; > > items = obj->package.elements; > + This… > switch (proptype) { > case DEV_PROP_U8: > ret = acpi_copy_property_array_u8(items, (u8 *)val, > nval); > @@ -552,8 +553,7 @@ int acpi_dev_prop_read(struct acpi_device *adev, > const char *propname, > ret = acpi_copy_property_array_string(items, (char > **)val, nval); > break; > default: > - ret = -EINVAL; > - break; > + return -EINVAL; …and this seem left overs. If you wish I can resend this patch without them. > } > return ret; > } -- Andy Shevchenko Intel Finland Oy