2012/4/8 Vojtech Horky <[email protected]>:
> Hi Adam.
>
> Dne 8. dubna 2012 15:47 Adam Saleh <[email protected]> napsal(a):
>>> Hi Adam,
>>> thanks for the patch.
>>>
>>> I have a few comments.
>>>
>>> Please, use the same C style as we do. In general, we are more
>>> generous with spaces than you were in your patch. We do put them
>>> around operators or after keywords (such as if or while).
>>
>> Ok, looked into the style guide, hope I fixed few things :)
> Unfortunately, not all of them. There are still some differences in
> your patch to our coding style. As I already mentioned, we do put
> spaces after if [e.g. if (cond) {, not if(cond){].
>>> And it seems to me that there is one bug in your implementation.
>>> Specifier %5.0f shall print the number without the fractional part.
>>> But your implementation prints at least one digit.
>>
>> Fixed, added a test.
> For me it does not work. The print2 test does not print two identical
> lines. Not talking about the fact that the printed format (first line)
> does not match the actual output.

Ok, I will fix the tests, I shouldn't have rushed so much :-)

> And AFAIK when the precision is not specified, it defaults to 6. At
> least as described in [1] and I can see no reason why we shall not
> follow it in this case.

You are right, I just checked K&R.  I based it on
http://www.cplusplus.com/ c library reference.

> And I encountered another problem. I tried to build your new patch for
> ia32 profile and I ended with following error:
> uspace/lib/c/generic/io/printf_core.c:559: undefined reference to 
> `__fixunsdfdi'
> This function is needed for 32bit architectures and it converts float
> to long long.

Ok, I will try to send you fix tomorrow.

> - Vojta
>
> [1] http://pubs.opengroup.org/onlinepubs/007908799/xsh/fprintf.html
>
>>
>>> Unless you want to provide your own fix, I will probably fix these
>>> some time next week because this is something I would like to have
>>> merged into mainline.
>>>
>>> Thanks.
>>>
>>> - Vojta
>>>
>>> Dne 7. dubna 2012 16:05 Adam Saleh <[email protected]> napsal(a):
>>>> Sending my patch. Hopefully fixes trac.helenos.org/ticket/221
>>>>
>>>> Adam
>>>>
>>>> _______________________________________________
>>>> 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
>>
>> _______________________________________________
>> 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
>

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

Reply via email to