| Message ID | 20210105202945.26913-1-fil.kolev@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1kwszi-002AFl-Fb; Tue, 05 Jan 2021 20:31:54 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S1730159AbhAEUbV (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Tue, 5 Jan 2021 15:31:21 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57286 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S1729608AbhAEUbV (ORCPT
<rfc822;linux-media@vger.kernel.org>); Tue, 5 Jan 2021 15:31:21 -0500
Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com
[IPv6:2a00:1450:4864:20::630])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D362DC061574;
Tue, 5 Jan 2021 12:30:40 -0800 (PST)
Received: by mail-ej1-x630.google.com with SMTP id w1so2161833ejf.11;
Tue, 05 Jan 2021 12:30:40 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=20161025;
h=from:to:cc:subject:date:message-id:mime-version
:content-transfer-encoding;
bh=ATMiMhOGdNGb2H7yTKxBixWgJ0m9iDQbo2lqGV5shds=;
b=QXiiDz3j1LH018diNgWoyn43TV7X/vca2nz7rpv6SszpgLBA3Y1LgBQ70HyLQur0xB
9wkS8SeeVfNsniBz+MjED0LH38zCm32YdsOupKiR23+mzmiEwvE1QtE4GX9uW5K3ImQI
yFVqs73aW955adpBQ1lAUuMHmG6VIzC73doJTUqUFXPfo3KiT8Ni/9RLvP9H1fzneiQj
bzVqkdzgfrEKujYNEeWuIdgNamxXdbsD4gBi2DiRcPGcbjadgNuFLIMCvbFS/rLhpPNl
tmaPgfJ83WT9VZGRhIAFuzdzA6dGKU+IHSVy4V4xCRKjLedvCcQc5JsyIDZf1GpuB+rx
LnHA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version
:content-transfer-encoding;
bh=ATMiMhOGdNGb2H7yTKxBixWgJ0m9iDQbo2lqGV5shds=;
b=UKdPKqXx22osegHUprJByRN/Of4h2ZlsWex7CnXbRPjScyuKdc23gdKw7J/5zLDSZJ
j6sGI3393KKEmrbxw9aCzqDPNFSGdHQhjQBZa6DGSUxoQCMXI0atHoftEHvHd0rH4bBc
b3KrGzrxT/xT1zkf/yjYqswojbeTPN0KbeHKvQY+73Y4rxBOWWEP69JHD2VLfB1U9aKT
bYOVzULev3DNVBlcFlAS9dr9/WrXMMGo2rvc8S1MhsmtLQe5s5h5Zx/i9QbAux2SIK2P
0oYEbTl4wLABjYL9+dmEemf16daACJooDxlAA8vXv9gJMiI6VrUekMlpgAifMXMJL5P9
1kbg==
X-Gm-Message-State: AOAM531877uW9GMQvnOQCG8hiWyzLx70HMPrO08zds6pwN4FCO02Vk3y
Pe+vL8oo1W/viQhUHNxVtR0=
X-Google-Smtp-Source:
ABdhPJxAiG2jqPLt0FbNfkA331PcVSFMVwN2ToZI78gYGShQZwmpRNVvIfGXSPCvFWQdBDRch2tVGw==
X-Received: by 2002:a17:907:20a4:: with SMTP id
pw4mr697060ejb.499.1609878639621;
Tue, 05 Jan 2021 12:30:39 -0800 (PST)
Received: from arch.laptop (84-238-151-182.ip.btc-net.bg. [84.238.151.182])
by smtp.gmail.com with ESMTPSA id j5sm79858edl.42.2021.01.05.12.30.38
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Tue, 05 Jan 2021 12:30:38 -0800 (PST)
From: Filip Kolev <fil.kolev@gmail.com>
Cc: Filip Kolev <fil.kolev@gmail.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-media@vger.kernel.org, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] media: atomisp: ov2722: replace hardcoded function name
Date: Tue, 5 Jan 2021 22:29:18 +0200
Message-Id: <20210105202945.26913-1-fil.kolev@gmail.com>
X-Mailer: git-send-email 2.30.0
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
To: unlisted-recipients:; (no To-header on input)
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -2.3 (--)
X-LSpam-Report: No,
score=-2.3 required=5.0 tests=BAYES_00=-1.9,DKIM_ADSP_CUSTOM_MED=0.001,DKIM_SIGNED=0.1,FREEMAIL_FORGED_FROMDOMAIN=0.001,FREEMAIL_FROM=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,T_DKIM_INVALID=0.01
autolearn=ham autolearn_force=no
|
| Series |
media: atomisp: ov2722: replace hardcoded function name
|
|
Commit Message
Filip Kolev
Jan. 5, 2021, 8:29 p.m. UTC
There is a debug message using hardcoded function name instead of the
__func__ macro. Replace it.
Report from checkpatch.pl on the file:
WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string
+ dev_dbg(&client->dev, "ov2722_remove...\n");
Signed-off-by: Filip Kolev <fil.kolev@gmail.com>
---
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > There is a debug message using hardcoded function name instead of the > __func__ macro. Replace it. > > Report from checkpatch.pl on the file: > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > + dev_dbg(&client->dev, "ov2722_remove...\n"); > > Signed-off-by: Filip Kolev <fil.kolev@gmail.com> > --- > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > index eecefcd734d0e..21d6bc62d452a 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) > struct v4l2_subdev *sd = i2c_get_clientdata(client); > struct ov2722_device *dev = to_ov2722_sensor(sd); > > - dev_dbg(&client->dev, "ov2722_remove...\n"); > + dev_dbg(&client->dev, "%s...\n", __func__); dev_dbg() provides the function name already, and this is just a "trace" call, and ftrace should be used instead, so the whole line should be removed entirely. thanks, greg k-h
On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: >> There is a debug message using hardcoded function name instead of the >> __func__ macro. Replace it. >> >> Report from checkpatch.pl on the file: >> >> WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string >> + dev_dbg(&client->dev, "ov2722_remove...\n"); >> >> Signed-off-by: Filip Kolev <fil.kolev@gmail.com> >> --- >> drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c >> index eecefcd734d0e..21d6bc62d452a 100644 >> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c >> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c >> @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) >> struct v4l2_subdev *sd = i2c_get_clientdata(client); >> struct ov2722_device *dev = to_ov2722_sensor(sd); >> >> - dev_dbg(&client->dev, "ov2722_remove...\n"); >> + dev_dbg(&client->dev, "%s...\n", __func__); > > dev_dbg() provides the function name already, and this is just a "trace" > call, and ftrace should be used instead, so the whole line should be > removed entirely. Thank you for the review! How do I go about this? Do I amend the patch and re-send as v2 or create a new patch entirely? Newbie here, doing this as part of the Eudyptula challenge, so I very much appreciate everyone's patience. > > thanks, > > greg k-h >
On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: > > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > > > There is a debug message using hardcoded function name instead of the > > > __func__ macro. Replace it. > > > > > > Report from checkpatch.pl on the file: > > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > > > + dev_dbg(&client->dev, "ov2722_remove...\n"); > > > > > > Signed-off-by: Filip Kolev <fil.kolev@gmail.com> > > > --- > > > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > index eecefcd734d0e..21d6bc62d452a 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) > > > struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > struct ov2722_device *dev = to_ov2722_sensor(sd); > > > - dev_dbg(&client->dev, "ov2722_remove...\n"); > > > + dev_dbg(&client->dev, "%s...\n", __func__); > > > > dev_dbg() provides the function name already, and this is just a "trace" > > call, and ftrace should be used instead, so the whole line should be > > removed entirely. > > Thank you for the review! > > How do I go about this? Do I amend the patch and re-send as v2 or create a > new patch entirely? New patch entirely please. thanks, greg k-h
On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote: > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > > > > There is a debug message using hardcoded function name instead of the > > > > __func__ macro. Replace it. > > > > > > > > Report from checkpatch.pl on the file: > > > > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > > > > + dev_dbg(&client->dev, "ov2722_remove...\n"); [] > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c [] > > > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) > > > > struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > > struct ov2722_device *dev = to_ov2722_sensor(sd); > > > > - dev_dbg(&client->dev, "ov2722_remove...\n"); > > > > + dev_dbg(&client->dev, "%s...\n", __func__); > > > > > > dev_dbg() provides the function name already, and this is just a "trace" > > > call, and ftrace should be used instead, so the whole line should be > > > removed entirely. > > > > Thank you for the review! > > > > How do I go about this? Do I amend the patch and re-send as v2 or create a > > new patch entirely? > > New patch entirely please. There are quite a lot of these relatively useless function tracing like uses in the kernel: $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l 1065 Perhaps yet another checkpatch warning would be useful: --- scripts/checkpatch.pl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e6857bdfcb2d..46b8ec8fe9e1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5981,6 +5981,14 @@ sub process { "Prefer using '\"%s...\", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); } +# check for unnecessary function tracing like uses + if ($rawline =~ /^\+\s*$logFunctions\s*\([^"]*"%s[\.\!]*\\n"\s*,\s*__func__\s*\)\s*;\s*$/) { + if (WARN("TRACING_LOGGING", + "Unnecessary ftrace-like logging - prefer using ftrace\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } # check for spaces before a quoted newline if ($rawline =~ /^.*\".*\s\\n/) { if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
On Wed, Jan 06, 2021 at 10:25:26AM -0800, Joe Perches wrote: > On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote: > > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: > > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > > > > > There is a debug message using hardcoded function name instead of the > > > > > __func__ macro. Replace it. > > > > > > > > > > Report from checkpatch.pl on the file: > > > > > > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > > > > > + dev_dbg(&client->dev, "ov2722_remove...\n"); > [] > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > [] > > > > > @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) > > > > > struct v4l2_subdev *sd = i2c_get_clientdata(client); > > > > > struct ov2722_device *dev = to_ov2722_sensor(sd); > > > > > - dev_dbg(&client->dev, "ov2722_remove...\n"); > > > > > + dev_dbg(&client->dev, "%s...\n", __func__); > > > > > > > > dev_dbg() provides the function name already, and this is just a "trace" > > > > call, and ftrace should be used instead, so the whole line should be > > > > removed entirely. > > > > > > Thank you for the review! > > > > > > How do I go about this? Do I amend the patch and re-send as v2 or create a > > > new patch entirely? > > > > New patch entirely please. > > There are quite a lot of these relatively useless function tracing like > uses in the kernel: > > $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l > 1065 These are printing other stuff besides just the function name. Maybe grep for '", __func__\)'? regards, dan carpenter
On Wed, 2021-01-06 at 22:36 +0300, Dan Carpenter wrote: > On Wed, Jan 06, 2021 at 10:25:26AM -0800, Joe Perches wrote: > > On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote: > > > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: > > > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > > > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > > > > > > There is a debug message using hardcoded function name instead of the > > > > > > __func__ macro. Replace it. > > > > > > > > > > > > Report from checkpatch.pl on the file: > > > > > > > > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > > > > > > + dev_dbg(&client->dev, "ov2722_remove...\n"); [] > > There are quite a lot of these relatively useless function tracing like > > uses in the kernel: > > > > $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l > > 1065 > > These are printing other stuff besides just the function name. No, these are printing _only_ the function name. > Maybe grep for '", __func__\)'? No, that wouldn't work as there are many many uses like: printk('%s: some string\n", __func__);
On Wed, Jan 06, 2021 at 01:17:47PM -0800, Joe Perches wrote: > On Wed, 2021-01-06 at 22:36 +0300, Dan Carpenter wrote: > > On Wed, Jan 06, 2021 at 10:25:26AM -0800, Joe Perches wrote: > > > On Wed, 2021-01-06 at 18:52 +0100, Greg Kroah-Hartman wrote: > > > > On Wed, Jan 06, 2021 at 07:43:42PM +0200, Filip Kolev wrote: > > > > > On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: > > > > > > On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: > > > > > > > There is a debug message using hardcoded function name instead of the > > > > > > > __func__ macro. Replace it. > > > > > > > > > > > > > > Report from checkpatch.pl on the file: > > > > > > > > > > > > > > WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string > > > > > > > + dev_dbg(&client->dev, "ov2722_remove...\n"); > [] > > > There are quite a lot of these relatively useless function tracing like > > > uses in the kernel: > > > > > > $ git grep -P '"%s[\.\!]*\\n"\s*,\s*__func__\s*\)' | wc -l > > > 1065 > > > > These are printing other stuff besides just the function name. > > No, these are printing _only_ the function name. > Oh... Duh... I was looking at the complete wrong output. My bad. Yeah. I like this warning. regards, dan carpenter
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c index eecefcd734d0e..21d6bc62d452a 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov2722_device *dev = to_ov2722_sensor(sd); - dev_dbg(&client->dev, "ov2722_remove...\n"); + dev_dbg(&client->dev, "%s...\n", __func__); dev->platform_data->csi_cfg(sd, 0); v4l2_ctrl_handler_free(&dev->ctrl_handler);