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... _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot