Re: [PATCH 4/12: eCryptfs] Main module functions

[Pekka Enberg <penberg@cs.helsinki.fi>]

Hi,

On 11/19/05, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> > > +       ecryptfs_printk(1, KERN_NOTICE, "Enter; lower_dentry = [%p], "
> > > +                       "lower_dentry->d_name.name = [%s], dentry = "
> > > +                       "[%p], dentry->d_name.name = [%s], sb = [%p]; "
> > > +                       "flag = [%.4x]; lower_dentry->d_count "
> > > +                       "= [%d]; dentry->d_count = [%d]\n", lower_dentry,
> > > +                       lower_dentry->d_name.name, dentry, dentry->d_name.name,
> > > +                       sb, flag, atomic_read(&lower_dentry->d_count),
> > > +                       atomic_read(&dentry->d_count));
> >
> > Could you use KERN_DEBUG instead and drop ecryptfs_printk()?

On Mon, 2005-11-21 at 10:10 -0600, Michael Thompson wrote:
> I don't follow with what you mean. We use ecryptfs_printk for 2 reasons:

Okay, let me spell it out for you: I think it is damn ugly :-)

> 1) Debugging. There is (or atleast at one time was) a large amount of
> ecryptfs_printk's scattered around during the development process.
> Obviously, much has been removed because we don't need them much
> anymore. However, the normal printk functionality doesn't, afaik,
> provide function & line number by default. ecryptfs_printk inserts it
> automatically for easy of use.

Like you said, not all debugging aids should be merged. I don't think
function and line number is enough an argument to justify putting your
own printk() in. That's my thinking anyway.

> 2) Verbosity switch. The intent, eventually atleast, is to be able to
> toggle the value of the verbosity level of ecryptfs at run time (this
> isn't implemented at the moment)... or atleast that's my plan :P Its
> not my project so I have to get "approval" etc. The 0th argument is
> the verbosity in which this message applies (1 being just informative,
> 0 being critical).

Do you really need it? Wouldn't it be better if you figured out which
debug printk() statements make sense and leave those in with KERN_DEBUG?

			Pekka