From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hin-Tak Leung Subject: Re: [PATCH 1/2] hfs/hfsplus: Convert dprint to hfs_dbg Date: Mon, 15 Apr 2013 01:53:01 +0100 (BST) Message-ID: <1365987181.4850.YahooMailClassic@web172306.mail.ir2.yahoo.com> References: <60b028ebc7235d8ba4ff1a2e936f879e0d806a9b.1365438494.git.joe@perches.com> Reply-To: htl10@users.sourceforge.net Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Vyacheslav Dubeyko , Joe Perches Return-path: Received: from nm3-vm0.bullet.mail.ird.yahoo.com ([77.238.189.213]:36301 "HELO nm3-vm0.bullet.mail.ird.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753394Ab3DOAxF convert rfc822-to-8bit (ORCPT ); Sun, 14 Apr 2013 20:53:05 -0400 In-Reply-To: <60b028ebc7235d8ba4ff1a2e936f879e0d806a9b.1365438494.git.joe@perches.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --- On Mon, 8/4/13, Joe Perches wrote: > Use a more current logging style. >=20 > Rename macro and uses. > Add do {} while (0) to macro. > Add DBG_ to macro. > Add and use hfs_dbg_cont variant where appropriate. >=20 > Signed-off-by: Joe Perches > +++ b/fs/hfs/hfs_fs.h > @@ -34,8 +34,18 @@ > //#define DBG_MASK=A0=A0=A0 > (DBG_CAT_MOD|DBG_BNODE_REFS|DBG_INODE|DBG_EXTENT) > #define DBG_MASK=A0=A0=A0 (0) > =20 > -#define dprint(flg, fmt, args...) \ > -=A0=A0=A0 if (flg & DBG_MASK) printk(fmt , ## > args) > +#define hfs_dbg(flg, fmt, ...)=A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 \ > +do {=A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 \ > +=A0=A0=A0 if (DBG_##flg & > DBG_MASK)=A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 \ > +=A0=A0=A0 =A0=A0=A0 printk(KERN_DEBUG > fmt, ##__VA_ARGS__);=A0=A0=A0 \ > +} while (0) > + > +#define hfs_dbg_cont(flg, fmt, ...)=A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 \ > +do {=A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 =A0=A0=A0 \ > +=A0=A0=A0 if (DBG_##flg & > DBG_MASK)=A0=A0=A0 =A0=A0=A0 > =A0=A0=A0 \ > +=A0=A0=A0 =A0=A0=A0 printk(KERN_CONT fmt, > ##__VA_ARGS__);=A0=A0=A0 \ > +} while (0) > + > =20 > /* > =A0 * struct hfs_inode_info This set of change seems to be somewhat zealous - it doesn't offer any = benefits other than possibly satisfying somebody's idea of code-purity. =46WIW, I have been sitting on a patch which changes this part of the c= ode to dynamic debugging, and it is much simplier. Just: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index e298b83..55d211d 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -45,8 +25,7 @@ #define HFSPLUS_JOURNAL_SWAP 1 =20 #define dprint(flg, fmt, args...) \ - if (flg & DBG_MASK) \ - printk(fmt , ## args) + pr_debug(fmt , ## args) =20 /* Runtime config options */ #define HFSPLUS_DEF_CR_TYPE 0x3F3F3F3F /* '????' */ =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D (and you can then remove all the DBG_* defines before that, since they = then don't have any effect any more). The benefit of this alternative is that it does not break any out-of-tr= ee patches, while make it easier to debug say patches... and I am still= sitting on a rather substantial set of the journal change, plus all th= e other issues that come out of it, like the folder count patch for cas= e-sensitive file systems. I think one needs to think very carefully about make bulk changes like = this, which serves no real purpose other than satisfying somebody's ide= a of code purity. The problem with such bulk "stylistic" changes, is that it forces peopl= e who are working on real functionalities and bug fixes to rebase their= work, and spend time on doing so, and also at the risk introducing new= bugs while rebasing. I know I am writing on a somewhat selfish purpose= : if I need to rebase my work due to other's bug fixes or enhancement, = etc, then fair enough, but I'd prefer not to rebase for the purpose of = other's preference of, and attempts at re-arranging the style of the de= bug statements, when the debugging output means little to them. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html