linux-kernel - Re: [PATCH 3/5] staging: r8188eu: (trivial) remove a duplicate debug print

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:   Sat, 14 Aug 2021 20:18:36 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Phillip Potter <phil@...lpotter.co.uk>
Cc:     Martin Kaiser <martin@...ser.cx>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Larry Finger <Larry.Finger@...inger.net>,
        Michael Straube <straube.linux@...il.com>,
        linux-staging@...ts.linux.dev,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/5] staging: r8188eu: (trivial) remove a duplicate debug print

On Saturday, August 14, 2021 6:54:40 PM CEST Phillip Potter wrote:
> On Fri, 13 Aug 2021 at 13:42, Fabio M. De Francesco
> 
> <fmdefrancesco@...il.com> wrote:
> > On Friday, August 13, 2021 12:05:36 PM CEST Martin Kaiser wrote:
> > > Hi Dan and Phil,
> > > [...] 
> > > > > Just my personal opinion, but I'd be inclined to strip out all 
DBG_88E
> > > > > calls totally. If there are necessary functions being called such as
> > > > > device_may_wakeup() we can always just keep this part and remove the
> > > > > macro call (not checked this function out myself yet). Thanks.
> > > 
> > > I'd agree with you, Phil. Most DBG_88E prints don't say anything useful.
> > > 
> > > This comment from Greg made me drop the DBG_88E removal for now
> > > 
> > > https://lore.kernel.org/linux-staging/20210803201511.29000-1-martin@kaiser.cx/T/#m05
> > > d82a 0ca8ed36180ebdc987114b4d892445c52d
> > 
> > Hi Martin,
> > 
> > I think you misunderstood what Greg was trying to convey with the above-
> > mentioned message.
> > 
> > Well, he doesn't like to feed developers with little spoons :-)
> > 
> > I'm pretty sure that, by "Why not use the proper debugging calls instead 
of
> > just deleting them?", he meant you should research, understand, and use 
the
> > proper APIs for printing debug messages.
> > 
> > Please check out pr_debug(), dev_dbg(), netdev_dbg(). Use them 
appropriately,
> > according to the subsystem you're working in and to the different types of
> > arguments they take.
> > 
> > Thanks,
> > 
> > Fabio
> > 
> > > A compromise would be to remove only those DBG_88E prints which are
> > > really not helpful.
> > > 
> > > Best regards,
> > > Martin
> 
> The problem I see is that this driver is so littered with unnecessary
> macro calls, how do we decide which ones to keep? In my mind, the
> better option is to remove them all and then come up with some new
> ones in the vein of netdev_dbg() and friends. I could be wrong of
> course :-) I tried going down the route of keeping/converting some to
> proper calls such as netdev_dbg() and the issue is a lot of the calls
> don't have an obvious value anyway.
> 
> Regards,
> Phil

I think that you'd better remove only the ones that "have no obvious value" 
and convert the others to using netdev_dbg(). Obviously, telling which have no 
value is at the discretion of whoever wants to carry on this work.

Regards,

Fabio