On Wed 12 Aug 2009 15:48, Scott Wood pondered:
On Wed, Aug 12, 2009 at 12:02:33PM +0200, Detlev Zundel wrote:
Hi Timur,
+#ifdef CONFIG_TFTP_QUIET +#define puts_quiet(fmt) +#else +#define puts_quiet(fmt) puts(fmt); +#endif
This looks backwards to me. I would do this:
#ifdef CONFIG_TFTP_QUIET #define puts(x) puts_quiet(x) #endif
That way, you don't need to change all of the puts calls to puts_quiet. Plus, having the normal calls be "puts_quiet" that changes to puts when QUIET is *not* enabled just feels wrong.
Just as a general remark - I consider it a bad idea to "overload" well known functions with non-standard behaviour. This breaks the "principle of least surprise" which turns out to be very valuable.
Technically, U-Boot's puts() is already non-standard (no automatic newline)...
But there's no redefinition in the original patch. It's just introducing a new puts_quiet().
Yeah, I think Detlev was just commenting on Timur's suggestion to the patch, not on the original patch...
I would be happy to change things to a debugX() - but this changes everyone's default behaviour (it becomes opt in, not opt-out), and most likely would cause some puzzlement when normally things don't print like they use to...