From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752676AbdHVUGt (ORCPT <rfc822;w@1wt.eu>);
        Tue, 22 Aug 2017 16:06:49 -0400
Received: from mout.web.de ([212.227.15.3]:53813 "EHLO mout.web.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752063AbdHVUGr (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 22 Aug 2017 16:06:47 -0400
Subject: Re: [PATCH] ALSA: via82xx: Use common error handling code in
 snd_via82xx_create()
To: Dan Carpenter <dan.carpenter@oracle.com>, alsa-devel@alsa-project.org
Cc: Jaroslav Kysela <perex@perex.cz>, Julia Lawall <Julia.Lawall@lip6.fr>,
        Takashi Iwai <tiwai@suse.com>, LKML <linux-kernel@vger.kernel.org>,
        kernel-janitors@vger.kernel.org
References: <2332472c-61d2-8c21-bf46-5c80745d6380@users.sourceforge.net>
 <20170822190757.76e5edh5t6tgvzpu@mwanda>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Message-ID: <d9b9dc7e-9c00-3f1d-f6f8-a2e67110c449@users.sourceforge.net>
Date: Tue, 22 Aug 2017 22:05:37 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170822190757.76e5edh5t6tgvzpu@mwanda>
Content-Type: text/plain; charset=utf-8
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
X-Provags-ID: V03:K0:btkzo4sjVyYwFA8XAbQ5rbaZEHYvgRURTbBAGPk1qm0nPUliFH0
 GdL8vFcXDM+IYuq15fPFKSt5h7UEAODj3FzgM82KX7xlH+S5lTMn2mVZA19CiZq8qX7auhB
 MxNdHRNJvLgOPcqJzlbvDrY+/+oxrF4oomeDs2ZXuu4219sqrCsckdFmuooaB/Ra4F3WkNV
 zIcTNucyZLwb4SvZ8UAiQ==
X-UI-Out-Filterresults: notjunk:1;V01:K0:RcfQVeUi77g=:qRownt2hrJjnNwgTzgg+TM
 1ldWDt1amRMjlg+AtE/ZHVL+OTEXrAjliTsFfgdtkW37+5/Mn4aNKYxsuGiEmhY+FLy91fDW1
 g1h35q+83rSVBFVabGY2Z2OVzYvIQZdjUvHl/0tKbQ775vWLe7phnFlX7IpPR8Vsk0pV/q0IN
 nFN5H6FBxmMbZ4z6XKgb7bjGWmLD65c5Ovfn/D8H9iXOIp0VB/Bt5CdzKTzrLcBovT0/iUuyx
 BeCHGcdv/PqiV/EjMe+kNaXBQKEm0sn7o9y3MP93spnPIlRN5lix1fW3LrUcVwVVs8sATltah
 qj52KxQbnVgthtNbWUmenGryun4UIasMOpodj68FJCxalXV8J8sKGoTYzDQGROKNffFZZ2NhW
 N3j9KWWOWK4lm002ApADeedgomyIsGPOnfLT4STrsD6GxqwEoNPfQrdaGy6rQR5urLH74/Rdi
 /3Xeq/Hdi5FRxs8VbzGvCFAjIAepIuQYawJNMautx15LRgkNaD7u2gPRBHuqOO3VtkWUWRcId
 wJT2eigDzL49o8elNCT2O2pYyBL2Qs4DYNliA89Pi4lWNZoKB37bi5vPmdz1kSePFjgEoDjsr
 cAnu8jrNVv3/FEbzAL12XHPAindjWb8SgzYeMDQQ5FAgl7v8ulHAFNOM/rjTpvdHU8A22uPRL
 gCOejkMNJ5os4aCVF02X0ye99QmRqRzwTiRYfk4RuF/xyg+PIreJVfnJ0naIUgC5r83MSo+1x
 q9m6tDyU3bY9l2VbpxjrsdE1vEuKV3mXJVriI6RHdFZRNvdRSelxrkNBNFNHL1Bns5IS8o8Xm
 DLl7W8lpvnHWOUFFWriKtytf3MsfwKiDbbsW6SNLjbmeWDPK2zXydxIxkdpsrjJHz1Init6
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>>  
>>  	if ((err = pci_request_regions(pci, card->driver)) < 0) {
>>  		kfree(chip);
>> -		pci_disable_device(pci);
>> -		return err;
>> +		goto disable_device;
>>  	}
> 
> [ snip ]
> 
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>  
>>  	*r_via = chip;
>>  	return 0;
>> +
>> +disable_device:
>> +	pci_disable_device(pci);
>> +	goto exit;
> 
> Why is this "goto exit" here?

Should the same return statement be reached after the proposed refactoring?

Would you like to move such a function call to this source code place?


> It's leaking now.

How do you come to this conclusion?


>> +free_chip:
>> +	snd_via82xx_free(chip);
>> +exit:
>> +	return err;
>>  }

Is this update suggestion worth for another look?

Regards,
Markus