From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752171AbbIXTIt (ORCPT ); Thu, 24 Sep 2015 15:08:49 -0400 Received: from lists.s-osg.org ([54.187.51.154]:39100 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751112AbbIXTIs (ORCPT ); Thu, 24 Sep 2015 15:08:48 -0400 Message-ID: <56044A37.6060909@osg.samsung.com> Date: Thu, 24 Sep 2015 20:08:39 +0100 From: Luis de Bethencourt User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: Takashi Iwai CC: linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, sudipm.mukherjee@gmail.com, joe@perches.com, perex@perex.cz, elfring@users.sourceforge.net Subject: Re: [PATCH v2] sound: oss: ad1848: Fix returned errno code in ad1848_init() References: <1443117510-26906-1-git-send-email-luisbg@osg.samsung.com> In-Reply-To: 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 On 24/09/15 19:56, Takashi Iwai wrote: > On Thu, 24 Sep 2015 19:58:30 +0200, > Luis de Bethencourt 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. > > Where is the propagated error number referred? In other words, do we > have a clear merit of changing this old stuff with rather a risk of > regression? > > > thanks, > > Takashi Hi Takashi, The propagated error number is not referred. The only merit here is to use the appropriate error number and silence the smatch tool. Thanks for the review, Luis > >> Changed the checks of the return of ad1848_init() to be for >= 0, >> instead of != -1. >> >> Smatch tool warning: >> ad1848_init() warn: returning -1 instead of -ENOMEM is sloppy >> >> Signed-off-by: Luis de Bethencourt >> --- >> sound/oss/ad1848.c | 4 ++-- >> sound/oss/dev_table.c | 2 +- >> sound/oss/pss.c | 2 +- >> 3 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/sound/oss/ad1848.c b/sound/oss/ad1848.c >> index 10c8de1..58c6d31 100644 >> --- a/sound/oss/ad1848.c >> +++ b/sound/oss/ad1848.c >> @@ -1992,7 +1992,7 @@ int ad1848_init (char *name, struct resource *ports, int irq, int dma_playback, >> portc = kmalloc(sizeof(ad1848_port_info), GFP_KERNEL); >> if(portc==NULL) { >> release_region(devc->base, 4); >> - return -1; >> + return -ENOMEM; >> } >> >> if ((my_dev = sound_install_audiodrv(AUDIO_DRIVER_VERSION, >> @@ -2007,7 +2007,7 @@ int ad1848_init (char *name, struct resource *ports, int irq, int dma_playback, >> { >> release_region(devc->base, 4); >> kfree(portc); >> - return -1; >> + return my_dev; >> } >> >> audio_devs[my_dev]->portc = portc; >> diff --git a/sound/oss/dev_table.c b/sound/oss/dev_table.c >> index 6dad515..abb3db4 100644 >> --- a/sound/oss/dev_table.c >> +++ b/sound/oss/dev_table.c >> @@ -148,7 +148,7 @@ EXPORT_SYMBOL(sound_install_mixer); >> >> void sound_unload_audiodev(int dev) >> { >> - if (dev != -1) { >> + if (dev >= 0) { >> DMAbuf_deinit(dev); >> audio_devs[dev] = NULL; >> unregister_sound_dsp((dev<<4)+3); >> diff --git a/sound/oss/pss.c b/sound/oss/pss.c >> index 81314f9..e84aafe 100644 >> --- a/sound/oss/pss.c >> +++ b/sound/oss/pss.c >> @@ -1091,7 +1091,7 @@ static int __init probe_pss_mss(struct address_info *hw_config) >> pss_mixer_reset(devc); >> attach_ms_sound(hw_config, ports, THIS_MODULE); /* Slot 0 */ >> >> - if (hw_config->slots[0] != -1) >> + if (hw_config->slots[0] >= 0) >> { >> /* The MSS driver installed itself */ >> audio_devs[hw_config->slots[0]]->coproc = &pss_coproc_operations; >> -- >> 2.5.1 >> >>