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@xxxxxxxxx> > > > --- > > > .../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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
