[SSSD] [PATCH][ding_libs] INI: Include missing header files

Lukas Slebodnik lslebodn at redhat.com
Thu Feb 13 16:05:55 UTC 2014


On (13/02/14 15:23), Michal Židek wrote:
>On 02/13/2014 01:40 PM, Lukas Slebodnik wrote:
>>On (13/02/14 10:55), Michal Židek wrote:
>>>On 01/17/2014 04:30 PM, 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
>>>>
>>>
>>>The patch itself is OK. Just a (very) small nitpick. Could you change
>>>the second part of patch description so that it is clear that you
>>>just added the declarations (from the description it looks like you
>>>wanted to include the header). I do not think anyone will actually
>>>read it and yes, it is obvious from the patch what you did, but if
>>>you fix this it will make me happy :)
>>>
>>
>>Previously, I included two header files, but there was an objection
>>@see
>>https://lists.fedorahosted.org/pipermail/sssd-devel/2014-January/018015.html
>>
>
>Yes, I read the thread and I am fine with the current patch. It is
>just that the current patch description looks to be a little misleading
>to me.
>
The patch in this thread is no longer valid, because it is incorporated into
another patch set.

LS



More information about the sssd-devel mailing list