On Fri, Feb 14, 2014 at 02:51:40PM +0100, Lukas Slebodnik wrote: > 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
I just pushed this one. > > > > > > >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 Pushed to ding-libs master. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
