linux-kernel - Re: [PATCH 2/6] device: Add #define dev_fmt similar to #define pr

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <54432c2a-a956-b721-0b2b-762a61898ae1@acm.org>
Date:   Fri, 6 Jul 2018 15:50:16 -0500
From:   Corey Minyard <minyard@....org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Joe Perches <joe@...ches.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] device: Add #define dev_fmt similar to #define pr_fmt

On 07/06/2018 10:50 AM, Greg Kroah-Hartman wrote:
> On Fri, Jul 06, 2018 at 08:41:34AM -0700, Joe Perches wrote:
>> On Fri, 2018-07-06 at 17:30 +0200, Greg Kroah-Hartman wrote:
>>> On Wed, May 09, 2018 at 08:15:46AM -0700, Joe Perches wrote:
>>>> Add a prefixing macro to dev_<level> uses similar to the pr_fmt
>>>> prefixing macro used in pr_<level> calls.
>>>>
>>>> This can help avoid some string duplication in dev_<level> uses.
>>>>
>>>> The default, like pr_fmt, is an empty #define dev_fmt(fmt) fmt
>>>>
>>>> Rename the existing dev_<level> functions to _dev_<level> and
>>>> introduce #define dev_<level> _dev_<level> macros that use the
>>>> new #define dev_fmt
>>>>
>>>> Miscellanea:
>>>>
>>>> o Consistently use #defines with fmt, ... and ##__VA_ARGS__
>>>> o Remove unnecessary externs
>>> SHouldn't these be separate patches please?
>> Multiple patches touching the same lines are unnecessary work.
>>
>> So I don't think so as it's just touching bits that need change
>> anyway.
>>
>>>> Signed-off-by: Joe Perches <joe@...ches.com>
>>>> ---
>>>>   drivers/base/core.c    |  12 +++---
>>>>   include/linux/device.h | 103 ++++++++++++++++++++++++++++---------------------
>>>>   2 files changed, 64 insertions(+), 51 deletions(-)
>>> Ok this seems like a lot of churn for no real apparent gain.  What is
>>> all of this getting us?
>>>
>>> What is the benifit, you have more code now,
>>> why is that good?
>> IPMI and a few other subsystems prefix all their output
>> dev_<level> with specific duplicated content.
> That's crazy and strange as the whole idea was that dev_* would uniquely
> identify the driver/device such that "prefixes" would not be needed.
>
> Ugh.
>
>> This centralizes those prefixes just like pr_fmt.
>>
>> The IPMI changes are already in mainline and now
>> need this mechanism to maintain their output content.
>>
>> IPMI and a few other subsystems prefix all their output
>> dev_<level> with specific duplicated content.
>>
>> This centralizes those prefixes just like pr_fmt.
>>
>> commit a2d70dfdda6f ("ipmi: msghandler: Add and use pr_fmt and dev_fmt,
>> remove PFX")
> Ok, I'll take this.  I don't like it, as drivers should really not need
> this at all.  But ipmi is known for doing "odd" things anyway...

I've been trying to make things less odd as time has gone by.  And I'm 
happy to
pull out this change.  I've been slowly working on the logging in the 
ipmi driver
to clean this kind of stuff up.

-corey

>
> greg k-h