On (14/02/14 08:40), Dmitri Pal wrote:
On 02/14/2014 04:19 AM, Lukas Slebodnik wrote:
>On (13/02/14 13:40), Dmitri Pal wrote:
>>On 02/13/2014 11:07 AM, Lukas Slebodnik wrote:
>>>+void print_config_parsing_errors(FILE *file,
>>>+ struct collection_item *error_set);
>>>+void print_file_parsing_errors(FILE *file,
>>>+ struct collection_item *error_list);
>>Seems like there is an extra tab or space on the second line.
>>I did a visual review. The patches otherwise look fine.
>>I have not tried to apply them though.
>Updated version is attached.
I applied patches, however I get warnings.
They might have been there, but since it is the area that you are
trying to clean it might make sense to not have these warnings:
[dpal@dpal ding-libs (master)]$ make check | grep warning
collection/collection_cnv.c:133: warning: no previous prototype for
collection/collection_cnv.c:387: warning: no previous prototype for
I remember a conversation about this issue...
I think we wanted to fix collection.h to have the right prototypes.
Fixing the prototypes is an ABI change however the functions are not
usable so no one uses it this way, thus I think it is safe just to
ABI is not changed. There was discussion only about API change.
and patch has already been ACK-ed, but it wasn't pushed to ding-libs master
ini/ini_config_ut.c:309: warning: ‘negative_test’ defined but not used
(error = negative_test()) ||
into ini_config_ut.c at line 1587 fixes the problem. I know we talked
about it. I now took a deeper look. I seems that you are right and
there is really not harm in adding this test.
The same situation here
patch has already been ACK-ed, but it wasn't pushed to ding-libs master
Otherwise ack. The patch to fix these things can be provided
separately on top of the patches you already sent.
Thank you very much for review.