linux-kernel - Re: [PATCH V4] mtd: Add DiskOnChip G3 support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 28 Sep 2011 14:45:47 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Robert Jarzmik <robert.jarzmik@...e.fr>
Cc:	dedekind1@...il.com, dwmw2@...radead.org,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4] mtd: Add DiskOnChip G3 support

On Tuesday 27 September 2011, Robert Jarzmik wrote:
> >
> > Really, passing the entire register name into an inline function is much
> > preferred over string concatenation, because it lets you grep for where
> > certain definitions are used.
> Right. But how do handle the "#seq" then ?
> 
> The whole point here is to write humanly readable debug messages. A message of
> the like "doc_flashsequence: 48 SET_PLANE1" is much better than
> "doc_flashsequence: 48", don't you think ?
> 
> If we consider to loose that debugging messages, it's way easier to drop the "do
> .. while(0)" sequences, and just keep the register write.
> 
> Now if you're convinced removing the "SET_PLANE1" debugging message is less
> important that ability to grep the full register name, why not, but from a
> maintenance point of view I would prefer letting the "macroness" in.

I generally recommend removing debug messages like this entirely from
production code. If you need them on production systems, that is an indication
that the code quality is not good enough. Enabling the debug output like
this also creates a lot of almost identical strings, which you don't need
if you turn it into an extern function that does

	static const char docseq[] = {
		[DOC_SEQ_RESET]		 = "reset",
		[DOC_SEQ_PAGE_SIZE_532]	 = "page_size_532",
	};
	dev_dbg(dev, "doc_flashSequence: %02x %s\n", docseq[seq]);

with exactly the same output.

Or you could turn the entire tracing into trace events and do the parsing
in user space, which seems appropriate if you frequently need to trace
these.

	Arnd
--
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/