Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751840AbcDOJr4 (ORCPT ); Fri, 15 Apr 2016 05:47:56 -0400 Received: from lists.s-osg.org ([54.187.51.154]:36721 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751585AbcDOJry (ORCPT ); Fri, 15 Apr 2016 05:47:54 -0400 Date: Fri, 15 Apr 2016 06:47:47 -0300 From: Mauro Carvalho Chehab To: Shuah Khan Cc: Shuah Khan , , , , , , , Subject: Re: [PATCH] media: saa7134 fix media_dev alloc error path to not free when alloc fails Message-ID: <20160415064747.2735370d@recife.lan> In-Reply-To: <57101729.1030909@samsung.com> References: <1460651480-6935-1-git-send-email-shuahkh@osg.samsung.com> <20160414180858.43c8620b@recife.lan> <57101729.1030909@samsung.com> Organization: Samsung X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1163 Lines: 44 Em Thu, 14 Apr 2016 16:18:17 -0600 Shuah Khan escreveu: > On 04/14/2016 03:08 PM, Mauro Carvalho Chehab wrote: > > Em Thu, 14 Apr 2016 10:31:20 -0600 > > Shuah Khan escreveu: > > > >> media_dev alloc error path does kfree when alloc fails. Fix it to not call > >> kfree when media_dev alloc fails. > > > > No need. kfree(NULL) is OK. > > Agreed. > > > > > Adding a label inside a conditional block is ugly. > > In this case, if label is in normal path, we will see defined, but not > used warnings when condition isn't defined. True, but we don't need a label here, as kfree() can be called with a null pointer. > We seem to have many such > cases for CONFIG_MEDIA_CONTROLLER :( We may try to address those media-controller dependent code latter on. I have some ideas of adding some macros and helper functions to allow getting rid of those ifdefs and not add extra code if !MEDIA_CONTROLLER, but the better seems to first add MC support to ALSA and make the enable/disable functions generic, and then cleanup the code to remove those ifdefs. > > thanks, > -- Shuah > > -- Thanks, Mauro