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!
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@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..
+struct ipa_options; +struct be_ctx;
Hmm, wouldn't it be nicer to also here include the headers that define/declare the structures..?
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@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?
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@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?
+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.
- 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@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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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@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@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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 06/04/2015 12:59 PM, Pavel Březina wrote: [snip]
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.
[snip]
OK, please see updated patches.
----- Original Message -----
From: "Pavel Reichl" preichl@redhat.com To: "Development of the System Security Services Daemon" sssd-devel@lists.fedorahosted.org Sent: Tuesday, June 2, 2015 7:25:53 PM Subject: [SSSD] [PATCH] dyndns: ipa_dyndns.h missed declaration of used data
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!
Ack from me.
On Thu, Jun 04, 2015 at 01:11:48PM -0400, Pavel Brezina wrote:
----- Original Message -----
From: "Pavel Reichl" preichl@redhat.com To: "Development of the System Security Services Daemon" sssd-devel@lists.fedorahosted.org Sent: Tuesday, June 2, 2015 7:25:53 PM Subject: [SSSD] [PATCH] dyndns: ipa_dyndns.h missed declaration of used data
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!
Ack from me.
Pushed to master: * 3683195b292afe77ce04ec446f3a1bb92d8876df * dd40f54bacdf18c6efd1ddf815251aa27b30d524
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Pavel Březina -
Pavel Reichl