[SSSD] [PATCHES][ding-libs] Enable extra compiler warning flags

Jakub Hrozek jhrozek at redhat.com
Wed Feb 19 10:22:09 UTC 2014


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 at 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.



More information about the sssd-devel mailing list