From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D7E6C83005 for ; Sat, 10 Jun 2023 17:10:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229555AbjFJRKZ (ORCPT ); Sat, 10 Jun 2023 13:10:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229512AbjFJRKU (ORCPT ); Sat, 10 Jun 2023 13:10:20 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A1F8E2D7E; Sat, 10 Jun 2023 10:10:19 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 359B6613F8; Sat, 10 Jun 2023 17:10:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E6B4AC433EF; Sat, 10 Jun 2023 17:10:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1686417018; bh=ZaO/RYVvXHSOs2jr3g0SgfMw80BeXu3HOAR90E2DWuM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=sBGyVra/pzwdhmhL3FVMM6WbG48PlQnnKFfNLANSLbHuIy7gjbiZFYaBJdmTvH+wd SbbDJ4ds5uQ8jhUfia44FtQSLf9rnu43Ujimj0K6MoRLjao28vbHUGSu/mwFL3BtYk ItJcJllgNWDny1ajlycFzqneBQQceocwuir3/FJMzZAxg9aTbhaATRR0mu54YjPAwX Lj6E4MRAxnOU6T+8rERqemrs44TP3KVcxc6mOgGmWtJmz+Jl6ZqTWEOp87TlBBnqYK FBXZGeZNqHnrljM25D+Yf/tGW0nczbauVgGAmCevqO5VY4cEEVbY3zsqCHBKVkNx+8 Lbem7Fx67r2fw== Date: Sat, 10 Jun 2023 19:10:15 +0200 From: Andi Shyti To: Krzysztof Kozlowski Cc: Christophe JAILLET , Alim Akhtar , Greg Kroah-Hartman , Jiri Slaby , Thomas Abraham , Kukjin Kim , linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-serial@vger.kernel.org Subject: Re: [PATCH 1/2] tty: serial: samsung_tty: Fix a memory leak in s3c24xx_serial_getclk() in case of error Message-ID: <20230610171015.vf7emd5crpr7n4mg@intel.intel> References: <20230610102607.7nonyh5xhuhpyy6e@intel.intel> <58d3f250-499d-5a18-6798-f9833cc2dbbd@wanadoo.fr> <20230610145429.uvmxxgxc5tc6x5b5@intel.intel> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jun 10, 2023 at 06:23:58PM +0200, Krzysztof Kozlowski wrote: > On 10/06/2023 16:54, Andi Shyti wrote: > > On Sat, Jun 10, 2023 at 04:07:51PM +0200, Christophe JAILLET wrote: > >> Le 10/06/2023 à 12:26, Andi Shyti a écrit : > >>>> @@ -1459,8 +1459,10 @@ static unsigned int s3c24xx_serial_getclk(struct s3c24xx_uart_port *ourport, > >>>> continue; > >>>> rate = clk_get_rate(clk); > >>>> - if (!rate) > >>>> + if (!rate) { > >>>> + clk_put(clk); > >>>> continue; > >>> > >>> could you also print an error here? > >>> > >> > >> Is: > >> dev_err(ourport->port.dev, > >> "Failed to get clock rate for %s.\n", clkname); > > Why do we need it? Most of other users of clk_get_rate() don't print. that's not a reason not to print it. > Probably because such condition is highly unlikely if not impossible. still... that's not a reason not to print it. All errors are unlikely and if it's unlikely, why there is no unlikely(!rate)? Which doesn't improve the reason not to print it. The more unlikely, the lauder you need to be: WARN_ON(!rate)... maybe too much! BUG_ON(!rate)... way too much! But these are inversely proportional to the likeliness of the error. > This makes simple function unnecessarily bigger... and... that's not a reason not to print it :) If it's needed, it's needed. If we are considering the error, then we need to treat it as an error. In any case, I'm not strong with it, indeed, I r-b it anyway. I personally prefer and suggested printing the error. Up to Christophe. Thanks, Andi