Hello, On Tue, Apr 03, 2012 at 08:30:54AM +0200, Vojtech Horky wrote: > Dne 2. dubna 2012 19:58 Sean Bartell <[email protected]> napsal(a): > > 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.
No problem. I committed a fix for all three problems. Thanks, Sean Bartell _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
