Hello Sean,

Dne 2. dubna 2012 19:58 Sean Bartell <[email protected]> napsal(a):
> Hello,
>
> On Mon, Apr 02, 2012, Vojtech Horky wrote:
>> Dne 2. dubna 2012 8:51 Sean Bartell <wingedtachikoma at gmail.com napsal(a):
>> > I've already made a few small improvements to HelenOS.
>> > lp:~wtachi/helenos/sleep implements bdsh/sleep and
>> I have looked at the patch and I have a few small comments.
>> * There is missing indentation in help_cmd_sleep() for the second
>> printf but this is really minor issue.
>
> Noted and fixed.
Thanks :-).

>
>> * Why is str_useconds_t declared static yet it is in the header file?
>> The purpose of static functions is to create functions private for
>> single source file.
>
> I imitated some of the other commands, like cat/cat.h. If they're wrong,
> I can fix them.
My fault, sorry. There are indeed static (not inlined) functions in
bdsh declared in headers. That is wrong, or at least it is not
consistent with the rest of the system. Thanks for pointing this out.

>
>> * And I do not like the str_useconds_t name much. IMHO the name shall
>> reflect that it parses input expressed in seconds (with fractions) but
>> returns usecs.
>
> I tried to make it consistent with str_uint<n>_t, but I could rename it
> to decimal_to_useconds.
I understand the motivation but due to this similarity, one might
expect that this parses useconds expressed as integer while it
actually parses floating seconds with useconds precision. It is not
that big deal - it is a private function and the function description
states that.

Thanks and sorry for that irresponsible fuss on the static functions.

- Vojta

>
> Thanks,
> Sean Bartell
>
> _______________________________________________
> HelenOS-devel mailing list
> [email protected]
> http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

Reply via email to