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. >>> >>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 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 https://lists.fedorahosted.org/pipermail/sssd-devel/2014-January/018013.html > > >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. The same situation here patch has already been ACK-ed, but it wasn't pushed to ding-libs master https://lists.fedorahosted.org/pipermail/sssd-devel/2014-January/018112.html > >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. LS _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
