linux-kernel - Re: [alsa-devel] Improving or replacing snd

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hr4gniri4.wl%tiwai@suse.de>
Date:	Fri, 31 May 2013 08:37:55 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Jaroslav Kysela <perex@...ex.cz>
Cc:	Alan Stern <stern@...land.harvard.edu>,
	Joe Perches <joe@...ches.com>, alsa-devel@...a-project.org,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [alsa-devel] Improving or replacing snd_printk()

At Thu, 30 May 2013 20:49:09 +0200,
Jaroslav Kysela wrote:
> 
> Date 30.5.2013 20:14, Alan Stern wrote:
> > On Wed, 29 May 2013, Joe Perches wrote:
> > 
> >> On Wed, 2013-05-29 at 15:27 -0400, Alan Stern wrote:
> >>> Has there been any thought of improving snd_printk(), or even better,
> >>> changing the call sites to use the dev_*() routines instead?
> >>
> >> Perhaps better would be to add and use some combination of
> >> type specific logging message functions/macros like:
> >>
> >> void snd_<level>(const struct snd_device *dev, const char *fmt, ...)
> >> void snd_card_<level>(const struct snd_card *card, const char *fmt, ...)
> >> void snd_ac97_<level>(const struct snd_ac97 *ac97, const char *fmt, ...)
> >>
> >> allowing NULL as the type pointer when necessary/appropriate.
> > 
> > Almost anything would be better than the current situation.  The only 
> > enhancement provided by CONFIG_SND_VERBOSE_PRINTK is to print the 
> > filename and line number, which is relatively unhelpful (unless there 
> > are multiple copies of the same message in several places).
> 
> I don't think that it's useless to see the origin of the source message
> from the first glance. It may be improved, of course, but adding more
> and more functions with minor benefits is another step to another hell.

Agreed.  Especially the debug messages would be better to be precise.

The problem is that many messages with snd_printk() like below have
too little information without CONFIG_SND_VERBOSE_PRINTK:
	snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err);

It's a problem only with snd_printk(), as CONFIG_SND_VERBOSE_PRINTK
influences on the behavior of snd_printk() and not on the debug prints
with snd_printd() & co.

Hence, the goal we should achieve is rather to drop
CONFIG_SND_VERBOSE_PRINTK.  *This* is the useless thing.

Meanwhile, many snd_printk() messages might start looking annoying
with the extra information.  If so, such lines should be replaced with
the standard prints like dev_*().  I think almost all snd_printk()
like below are better replaced with standard ones.

So, alternatively, we can begin with replacing some snd_printk() with
the standard functions, then dropping CONFIG_SND_VERBOSE_PRINTK.

I think most of snd_printd() and snd_printdd() can be kept as is.
These are just debug messages, after all.

Thoughts?


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/