lkml.org 
[lkml]   [2015]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init
    On Wed, May 20, 2015 at 10:26:30AM +0200, pmarzo wrote:
    > On mié, 2015-05-20 at 00:46 +0300, Dan Carpenter wrote:
    > > I was planning to leave this one for Greg but since you asked me to
    > > comment...
    > >
    > > This patch is ok as one patch. The subject is "clean up
    > > prism2_wep_init()" and that's one thing. Breaking it up into tiny
    > > patches would probably make it harder to review.
    > >
    > > On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote:
    > > > Merge two pr_debug lines with literal strings splitted across several lines
    > > > into one single line, simplifying prism2_wep_init error check code.
    > > >
    > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
    > > > ---
    > > > .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++-------------
    > > > 1 file changed, 9 insertions(+), 13 deletions(-)
    > > >
    > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
    > > > index 0a17f84..388ed3c 100644
    > > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
    > > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
    > > > @@ -9,6 +9,8 @@
    > > > * more details.
    > > > */
    > > >
    > > > +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt
    > >
    > > Greg doesn't like pr_fmt() in driver code, use dev_dbg() and friends
    > > instead. I don't like debug output generally so I say just delete it.
    > > Especially in this case the debug output is pretty useless.
    > dev_dbg needs the device parameter, I don't see a way to get the device
    > pointer within this function. Anyway I think you are right, this debug
    > output is not very useful, if Greg agrees I will delete the line.

    Greg doesn't care. Don't wait for Greg. He is a busy guy.

    Greg is very predictable so I can normally tell you which patches will
    be merged and which will not.

    > I am curious, why you dont like debug output? I usually have to adapt
    > the kernel to run in ARM based boards and I find debug output really
    > useful.

    These debug statements are never going to be printed in real life. They
    are a waste of RAM for no reason. They make the code more complicated
    which is why we are debating "how should we handle this thing?" They
    make the code slightly messier. They can introduce NULL deref bugs in
    code which never gets tested.

    It's not that all debug code is bad, but so often people do it without
    thinking because they think it's something they must do for every
    function. They think they are doing the right thing without realizing
    that sometimes they are making the code worse.

    Low level functions like crypto_alloc_blkcipher() should have debug code
    and stack dumps on error. It mostly does I think. I think it assumes
    that /lib/modules/ is not corrupt...

    regards,
    dan carpenter



    \
     
     \ /
      Last update: 2015-05-20 11:21    [W:2.221 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site