Hi Mike,
On Sun, Feb 26, 2012 at 8:35 PM, Mike Frysinger <vap...@gentoo.org> wrote:
> On Sunday 26 February 2012 23:26:52 Simon Glass wrote:
>> On Sun, Feb 26, 2012 at 8:20 PM, Mike Frysinger wrote:
>> > On Sunday 26 February 2012 21:46:23 Simon Glass wrote:
>> >> On Sun, Feb 26, 2012 at 2:38 PM, Mike Frysinger wrote:
>> >> > int main(int argc, char *argv[])
>> >> > {
>> >> > ...
>> >> > + state = state_get_current();
>> >> > + os_parse_args(state, argc, argv);
>> >>
>> >> We don't check the return value. Perhaps add a comment as to why.
>> >
>> > since the code takes care of setting parse_err itself now, i'm not sure
>> > what to do with the return value
>>
>> I agree it is right, just asking for a comment. Same with most of my
>> other things - more comments as I suggested is nice for people that
>> come into the code fresh.
>
> my plan was to clean this up a bit more before submitting (such as adding
> comments). i was looking more for feedback on the general approach here and
> any fundamental sticking points since this is a semi-radical departure from
> what either of us posted earlier.
OK I see. Well actually I was expecting that we would need something
along these lines eventually, since arg parsing belongs with the
module that provides the argument I think. So as well to have it now,
even if it doesn't have a lot of uses yet. It will.
>
> in this case, i'm asking: should we just toss the return value and have it be
> void ?
I suggest not - even if we ignore it, it seems like information that
should be returned. Perhaps we should make it more explicit by
returning state->return_code with a comment that callers can use it
now or later. But I think the way you have done it is fine.
Regards,
Simon
> -mike
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
