Re: [PATCH] staging: dgap: fix returned errno code in dgap_parsefile()
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dgap: fix returned errno code in dgap_parsefile()
On Tue, Sep 22, 2015 at 08:38:43AM +0200, Javier Martinez Canillas wrote: > 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? > > 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. Yes. I just had a doubt what you meant by caller. If user then I would have said that "patch is correct but commit message is not". :) regards sudip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dgap: fix returned errno code in dgap_parsefile()
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? 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(). regards sudip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
