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.
Fixed. Updated version is attached.
LS
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 ‘col_insert_unsigned_property’ collection/collection_cnv.c:387: warning: no previous prototype for ‘col_insert_unsigned_property_with_ref’
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 fix: s/unsinged/unsigned
in collection.h
ini/ini_config_ut.c:309: warning: ‘negative_test’ defined but not used
adding (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.
Otherwise ack. The patch to fix these things can be provided separately on top of the patches you already sent.