From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756852AbbIVGiu (ORCPT ); Tue, 22 Sep 2015 02:38:50 -0400 Received: from lists.s-osg.org ([54.187.51.154]:38539 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750775AbbIVGit (ORCPT ); Tue, 22 Sep 2015 02:38:49 -0400 Subject: Re: [PATCH] staging: dgap: fix returned errno code in dgap_parsefile() To: Sudip Mukherjee References: <1442882376-23634-1-git-send-email-javier@osg.samsung.com> <20150922045238.GA5526@sudip-pc> From: Javier Martinez Canillas X-Enigmail-Draft-Status: N1110 Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Greg Kroah-Hartman , Daeseok Youn , driverdev-devel@linuxdriverproject.org, Lidza Louina Message-ID: <5600F773.5040208@osg.samsung.com> Date: Tue, 22 Sep 2015 08:38:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <20150922045238.GA5526@sudip-pc> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Sudip, On 09/22/2015 06:52 AM, Sudip Mukherjee wrote: > On Tue, Sep 22, 2015 at 02:39:36AM +0200, Javier Martinez Canillas wrote: >> The driver is using -1 instead of the -ENOMEM defined macro to specify >> that a buffer allocation failed. Since the error number is propagated, >> the caller will get a -EPERM which is the wrong error condition. > Just a little doubt. caller means the function which is calling this > dgap_parsefile() or you meant the user? I meant whatever function calls dgap_parsefile(), which currently is only dgap_firmware_load(). > The function which is calling this dgap_parsefile() is just checking if > it has received 0 or something else. Something else is error and it > rerturns -EINVAL for all types of error (ofcourse that is also wrong). > So the user will see -EINVAL for all types of error in dgap_parsefile(). > Yes, I also verified what dgap_firmware_load() does with the returned error code to make sure that it was safe to do this change without affecting the rest of the driver. But I believe the patch and what the commit message says is true regardless of the fact that the caller is just checking for != 0. dgap_firmware_load() stills gets a wrong error condition whether it's checking it or not. > regards > sudip > Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America