[SSSD] [PATCH] dyndns: ipa_dyndns.h missed declaration of used data

Pavel Březina pbrezina at redhat.com
Thu Jun 4 10:59:04 UTC 2015


On 06/03/2015 11:07 AM, Pavel Reichl wrote:
>
>
> On 06/02/2015 08:23 PM, Jakub Hrozek wrote:
>> On Tue, Jun 02, 2015 at 07:25:53PM +0200, Pavel Reichl wrote:
>>>> There are some other issues with the header file, if you want to go
>>>> wild
>>>> -- it uses be_ctx but doesn't bring in the API that declares be_ctx.
>>>> Same for ipa_options and errno_t.
>>> Hello,
>>>
>>> please see simple patch attached.
>>>
>>> Thanks!
>>>  From 42bf9fca1151e7392292dca94b2cb5862ddb25d2 Mon Sep 17 00:00:00 2001
>>> From: Pavel Reichl <preichl at redhat.com>
>>> Date: Tue, 2 Jun 2015 13:08:56 -0400
>>> Subject: [PATCH 1/2] dyndns: ipa_dyndns.h missed declaration of used
>>> data
>>>
>>> ipa_dyndns.h was depended on header files included before it.
>>> ---
>>>   src/providers/ipa/ipa_dyndns.h | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/providers/ipa/ipa_dyndns.h
>>> b/src/providers/ipa/ipa_dyndns.h
>>> index
>>> a03e6e22932c24995bb4bc3090076cb6f047f9d9..d76ac59ca6d16022badc1cd2be04055a452cb8d6
>>> 100644
>>> --- a/src/providers/ipa/ipa_dyndns.h
>>> +++ b/src/providers/ipa/ipa_dyndns.h
>>> @@ -25,6 +25,11 @@
>>>   #ifndef IPA_DYNDNS_H_
>>>   #define IPA_DYNDNS_H_
>>> +#include "util/util_errors.h"
>> For errno, it's OK to just include errno.h. I doubt errno_t is going to
>> move..
>
> I'm sorry I don't understand - errno.h as a standard lib header file? We
> don't have any our errno.h file, right?
> I see errno_t redefined in hbac_evaluator.c, nss_mc.h, sss_cli.h,
> util_errors.h do you prefer if I redefine it also in ipa_dyndns.h?

I think Jakub just misspelled the header file. Including util_errors.h 
seems to be valid.

>
>>
>>> +
>>> +struct ipa_options;
>>> +struct be_ctx;
>> Hmm, wouldn't it be nicer to also here include the headers that
>> define/declare the structures..?
> I did it just to minimize compile times as sources including
> ipa_dyndns.h won't need to be recompiled if any of ipa_common.h,
> dp_backend.h and their nested header files get changed (unless they are
> already included of course).  I also believe it makes dependencies of
> source file more obvious. But I really don't insist. I gladly change it.

Those header files don't change that often so I don't think you're 
argument is reasonable. Please, include headers that are required instead.

>>
>>> +
>>>   void ipa_dyndns_update(void *pvt);
>>>   void ipa_dyndns_timer(void *pvt);
>>> --
>>> 2.1.0
>>>
>>>  From 0da03c3ea44f7de403bf3cfa728105645c6a84e1 Mon Sep 17 00:00:00 2001
>>> From: Pavel Reichl <preichl at redhat.com>
>>> Date: Tue, 2 Jun 2015 13:12:43 -0400
>>> Subject: [PATCH 2/2] krb: remove duplicit decl. of write_krb5info_file
>>>
>>> function write_krb5info_file() was declared twice in krb5_common.h
>>> ---
>>>   src/providers/krb5/krb5_common.h | 3 ---
>>>   1 file changed, 3 deletions(-)
>>>
>>> diff --git a/src/providers/krb5/krb5_common.h
>>> b/src/providers/krb5/krb5_common.h
>>> index
>>> 8f6fb229ac2ecf83ceec67d3b0817ddd57b93543..34c0eb96020f97cbd8ee75624c31f9d45649d14a
>>> 100644
>>> --- a/src/providers/krb5/krb5_common.h
>>> +++ b/src/providers/krb5/krb5_common.h
>>> @@ -181,9 +181,6 @@ errno_t krb5_install_offline_callback(struct
>>> be_ctx *be_ctx,
>>>   errno_t krb5_install_sigterm_handler(struct tevent_context *ev,
>>>                                        struct krb5_ctx *krb5_ctx);
>>> -errno_t write_krb5info_file(const char *realm, const char *kdc,
>>> -                            const char *service);
>>> -
>> ACK
>>
>> I wonder how are we creating these bugs, bad patch merge?
> I suppose so. I did some grepping and this was the only remaining
> "double declarated" function.
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list