linux-kernel - Re: [PATCH v7 03/15] drivers: thermal: tsens: Add __func_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 1 Nov 2019 15:53:26 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Amit Kucheria <amit.kucheria@...aro.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Eduardo Valentin <edubezval@...il.com>,
        Andy Gross <agross@...nel.org>, masneyb@...tation.org,
        Stephen Boyd <swboyd@...omium.org>, julia.lawall@...6.fr,
        Amit Kucheria <amit.kucheria@...durent.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v7 03/15] drivers: thermal: tsens: Add __func__ identifier
 to debug statements

Hi,

On Thu, Oct 31, 2019 at 11:38 AM Amit Kucheria <amit.kucheria@...aro.org> wrote:
>
> Printing the function name when enabling debugging makes logs easier to
> read.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@...aro.org>
> Reviewed-by: Stephen Boyd <swboyd@...omium.org>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
>  drivers/thermal/qcom/tsens-common.c | 8 ++++----
>  drivers/thermal/qcom/tsens.c        | 6 +++---
>  2 files changed, 7 insertions(+), 7 deletions(-)

Obviously my opinion doesn't trump maintainers ones, but I have always
heard that this is the wrong thing to do for "dev_xxx" debug
statements.  Specifically:

* For "dev_xxx" statements, the device name is already printed which
is pretty good for getting you to the right driver.

* Once you're in the right driver, error messages should be unique
enough to find the right function.

If having __func__ in all messages was beneficial then the "dev_xxx"
macros would include it by default.  They don't and such things just
uglify the logs and chew up log space.  I suppose you could try
including a CONFIG option to add it to all "dev_xxx" functions and see
if it would be accepted?  Then you can turn it on locally.

Using __func__ in cases where you don't happen to have a "struct
device" (and can't get one easily) makes some sense, but otherwise I
believe it should be kept out.

-Doug