[U-Boot] [PATCH v2 2/2] Add a cli command to test the TPM device.

[Vadim Bendebury]

Dear Wolfgang Denk,

On Sat, Oct 15, 2011 at 12:44 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Vadim Bendebury,
>
> In message <CAC3GErHaAGX39XjD04MnJWe3sa9XC087LLpf6SycVC6K7SLt6Q at mail.gmail.com> you wrote:
>>
>> >> + * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
>> >> + * Released under the 2-clause BSD license.
>> >
>> > Are we ok with this ? Also, you say something about GPL in the same comment?
>> >
>>
>> Can someone please tell me what needs to be put in the license headers
>> and I will do it. I hear different suggestions from different people.
>
> See previous comment - drop the BSD part if you include a GPLv2+
> license header.
>

done

>> >> +             return ~0;
>> >
>> > Ugh, return 1 isn't ok ? Using ~0 on int type is weird.
>>
>> I was under impression that any nonzero value is good. I see sometimes
>> -1 returned for error in other u-boot sources. Also, I am sorry, I am
>> new to this, when someone says "it is weird" - does this mean that  it
>> has to be changed?
>
> Commands are running in some sort of shell environment.  SO please
> return 0 for OK and 1 for general errors like all other commands do
> (or should do).
>

done

> ...
>> >> +static void report_error(const char *msg)
>> >> +{
>> >> +   if (msg && *msg)
>> >
>> > Uhm, you also check if first character is non-zero? why ?
>>
>> To avoid printing an empty string if someone calls this with an empty message?
>
> It's your code, so just don't do it, then.
>
> And what's wrong about printing an empty string?  YOuy're just adding
> dead code (and increased memory footprint) here.
>
>> > Two underscores aren't a good practice.
>>
>> I did this as a result of a previous review. Do you have a suggestion
>> how this should be done instead?
>
> First, and most important, __u_boot_cmd_tpm appears to be undefined in
> your two patches, so it looks to be a real bug.
>
> Second, please read the C standard's section about reserved
> identifiers.
>


reworked to avoid all the complications.

cheers,
/vb

> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> The universe contains any amount of horrible ways  to  be  woken  up,
> such as the noise of the mob breaking down the front door, the scream
> of fire engines, or the realization that today is the Monday which on
> Friday night was a comfortably long way off.
>                                 - Terry Pratchett, _Moving Pictures_
>