Hi,
> 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...
Correct.
Cheers
Detlev
--
To you I'm an atheist; to God, I'm the Loyal Opposition.
-- Woody Allen
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
