On (17/01/14 16:30), Lukas Slebodnik wrote: >On (13/01/14 10:39), Dmitri Pal wrote: >>On 01/13/2014 09:54 AM, Lukas Slebodnik wrote: >>> On (13/01/14 09:07), Dmitri Pal wrote: >>>> On 01/13/2014 05:49 AM, Lukas Slebodnik wrote: >>>>> ehlo, >>>>> >>>>> Function ini_config_serialize was declared in the header file >>>>> ini_configobj.h, >>>>> but this header file was not included in implementation module >>>>> ini_serialize.c >>>> This is fine though I do not see a big reason to do this. >>>> >>>>> Functions print_config_parsing_errors, print_file_parsing_errors were >>>>> declared >>>>> in the header file ini_config.h, but this header file was not included >>>>> in implementation module ini_print.c >>>> ini_config.h is the old interface >>>> ini_configobj.h is the new interface >>> Yes, >>> but problem is that implementations of functions from old interface are in >>> the >>> same module (ini_print.c) like implementations of functions from old >>> interface. >>> If you do not like including both header files in one module, we should >>> file a >>> ticket to split file ini_print.c. It is internal change and I hope easy >>> task. >>> >>> But without this patch there will be some warnings: >>> CC ini_print.lo >>> ini/ini_print.c:327:6: warning: no previous prototype for >>> 'print_file_parsing_errors' [-Wmissing-prototypes] >>> void print_file_parsing_errors(FILE *file, >>> ^ >>> ini/ini_print.c:405:6: warning: no previous prototype for >>> 'print_config_parsing_errors' [-Wmissing-prototypes] >>> void print_config_parsing_errors(FILE *file, >>> ^ >> >>I took a look at these two functions. They are not used in the new >>interface. >>I suggest we just explicitly add declarations for these two functions to >>the top of ini_print.c instead of including the header file. >Done > >>Splitting the module is a more labor intensive change. I would rather >>wait some time and just remove the old interface. >>Then split would not be needed. >> >>>> These headers should not be included into one module. >>>> They are mutually exclusive. >>>> >>>> Functions should probably be included into ini_configobj.h but I have >>>> not included them because they are already declared in the old interface. >>>> So may be we should move them from old to new rather than declare them >>>> twice? >>>> > >New file is attached. > >LS
>From 67f7ebea952af2cce671ae338a09d2f13677ff40 Mon Sep 17 00:00:00 2001 >From: Lukas Slebodnik <lsleb...@redhat.com> >Date: Sat, 11 Jan 2014 14:29:53 +0100 >Subject: [PATCH] INI: Include missing header file with declarations > >Function ini_config_serialize was declared in the header file ini_configobj.h, >but this header file was not included in implementation module ini_serialize.c > >Functions print_config_parsing_errors, print_file_parsing_errors were declared >in the header file ini_config.h, but this header file was not included >in implementation module ini_print.c >--- The patch expects some volunteer for review. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
