6:59 a.m.
Hello,
please see attached simple patch. I believe that nested header files are
sometimes necessary evil, but they should be avoided when possible. It
makes difficult figuring out on which header files source file actually
depend on and can increase compilation time.
Thanks.
7:15 a.m.
On (19/05/15 13:59), Pavel Reichl wrote:
Hello,
please see attached simple patch. I believe that nested header files are
sometimes necessary evil, but they should be avoided when possible. It makes
difficult figuring out on which header files source file actually depend on
and can increase compilation time.
Thanks.
From d11f9e4538cbb6d247f43e427347d59e1ca6c98f Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 19 May 2015 07:35:11 -0400
Subject: [PATCH] util: don't include util.h in src/util/authtok.h
---
src/util/authtok.c | 1 +
src/util/authtok.h | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/authtok.c b/src/util/authtok.c
index 45761df80175fded8a6c6e5dac8a90180b11d225..784512c0bd21128bd16d3663bf460a286e8b3eed
100644
--- a/src/util/authtok.c
+++ b/src/util/authtok.c
@@ -17,6 +17,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>;.
*/
+#include "util/util.h"
#include "authtok.h"
struct sss_auth_token {
diff --git a/src/util/authtok.h b/src/util/authtok.h
index cb366270832852281a222018f8e27feb1500ff01..d82168f5823e188e5c04fa418cb0ff52f6a52aac
100644
--- a/src/util/authtok.h
+++ b/src/util/authtok.h
@@ -20,7 +20,6 @@
#ifndef __AUTHTOK_H__
#define __AUTHTOK_H__
-#include "util/util.h"
#include "util/authtok-utils.h"
#include "sss_client/sss_cli.h"
This is not right solution. Even thought everything compiles without any
problem.
"util/util.h" was included because errnot_t is used in prototypes of functions.
sh$ grep -n errno_t src/util/authtok.h
70:errno_t sss_authtok_get_password(struct sss_auth_token *tok,
85:errno_t sss_authtok_set_password(struct sss_auth_token *tok,
101:errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
115:errno_t sss_authtok_set_ccfile(struct sss_auth_token *tok,
140:errno_t sss_authtok_set(struct sss_auth_token *tok,
154:errno_t sss_authtok_copy(struct sss_auth_token *src,
198:errno_t sss_authtok_set_2fa(struct sss_auth_token *tok,
222:errno_t sss_authtok_get_2fa(struct sss_auth_token *tok,
and with your patch "errno_t" has to be defined before including authtok.h
It's better to do not rely on the order of included header files.
LS
7:25 a.m.
On 05/19/2015 02:15 PM, Lukas Slebodnik wrote:
On (19/05/15 13:59), Pavel Reichl wrote:
> Hello,
>
> please see attached simple patch. I believe that nested header files are
> sometimes necessary evil, but they should be avoided when possible. It makes
> difficult figuring out on which header files source file actually depend on
> and can increase compilation time.
>
> Thanks.
>From d11f9e4538cbb6d247f43e427347d59e1ca6c98f Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Tue, 19 May 2015 07:35:11 -0400
> Subject: [PATCH] util: don't include util.h in src/util/authtok.h
>
> ---
> src/util/authtok.c | 1 +
> src/util/authtok.h | 1 -
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/authtok.c b/src/util/authtok.c
> index
45761df80175fded8a6c6e5dac8a90180b11d225..784512c0bd21128bd16d3663bf460a286e8b3eed 100644
> --- a/src/util/authtok.c
> +++ b/src/util/authtok.c
> @@ -17,6 +17,7 @@
> along with this program. If not, see <http://www.gnu.org/licenses/>;.
> */
>
> +#include "util/util.h"
> #include "authtok.h"
>
> struct sss_auth_token {
> diff --git a/src/util/authtok.h b/src/util/authtok.h
> index
cb366270832852281a222018f8e27feb1500ff01..d82168f5823e188e5c04fa418cb0ff52f6a52aac 100644
> --- a/src/util/authtok.h
> +++ b/src/util/authtok.h
> @@ -20,7 +20,6 @@
> #ifndef __AUTHTOK_H__
> #define __AUTHTOK_H__
>
> -#include "util/util.h"
> #include "util/authtok-utils.h"
> #include "sss_client/sss_cli.h"
This is not right solution. Even thought everything compiles without any
problem.
"util/util.h" was included because errnot_t is used in prototypes of
functions.
sh$ grep -n errno_t src/util/authtok.h
70:errno_t sss_authtok_get_password(struct sss_auth_token *tok,
85:errno_t sss_authtok_set_password(struct sss_auth_token *tok,
101:errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
115:errno_t sss_authtok_set_ccfile(struct sss_auth_token *tok,
140:errno_t sss_authtok_set(struct sss_auth_token *tok,
154:errno_t sss_authtok_copy(struct sss_auth_token *src,
198:errno_t sss_authtok_set_2fa(struct sss_auth_token *tok,
222:errno_t sss_authtok_get_2fa(struct sss_auth_token *tok,
and with your patch "errno_t" has to be defined before including authtok.h
It's better to do not rely on the order of included header files.
Yes, you are
right, thanks.
I suppose that correct solution would be then include just
'util/util_errors.h' in src/util/authtok.h. But I suppose we have
bigger fish to fry...or do you encourage me to send such a patch?
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
10:26 a.m.
On 05/19/2015 02:25 PM, Pavel Reichl wrote:
On 05/19/2015 02:15 PM, Lukas Slebodnik wrote:
> On (19/05/15 13:59), Pavel Reichl wrote:
>> Hello,
>>
>> please see attached simple patch. I believe that nested header files
>> are
>> sometimes necessary evil, but they should be avoided when possible.
>> It makes
>> difficult figuring out on which header files source file actually
>> depend on
>> and can increase compilation time.
>>
>> Thanks.
> >From d11f9e4538cbb6d247f43e427347d59e1ca6c98f Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl(a)redhat.com>
>> Date: Tue, 19 May 2015 07:35:11 -0400
>> Subject: [PATCH] util: don't include util.h in src/util/authtok.h
>>
>> ---
>> src/util/authtok.c | 1 +
>> src/util/authtok.h | 1 -
>> 2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/util/authtok.c b/src/util/authtok.c
>> index
>>
45761df80175fded8a6c6e5dac8a90180b11d225..784512c0bd21128bd16d3663bf460a286e8b3eed
>> 100644
>> --- a/src/util/authtok.c
>> +++ b/src/util/authtok.c
>> @@ -17,6 +17,7 @@
>> along with this program. If not, see
>> <http://www.gnu.org/licenses/>;.
>> */
>>
>> +#include "util/util.h"
>> #include "authtok.h"
>>
>> struct sss_auth_token {
>> diff --git a/src/util/authtok.h b/src/util/authtok.h
>> index
>>
cb366270832852281a222018f8e27feb1500ff01..d82168f5823e188e5c04fa418cb0ff52f6a52aac
>> 100644
>> --- a/src/util/authtok.h
>> +++ b/src/util/authtok.h
>> @@ -20,7 +20,6 @@
>> #ifndef __AUTHTOK_H__
>> #define __AUTHTOK_H__
>>
>> -#include "util/util.h"
>> #include "util/authtok-utils.h"
>> #include "sss_client/sss_cli.h"
> This is not right solution. Even thought everything compiles without any
> problem.
>
> "util/util.h" was included because errnot_t is used in prototypes of
> functions.
>
> sh$ grep -n errno_t src/util/authtok.h
> 70:errno_t sss_authtok_get_password(struct sss_auth_token *tok,
> 85:errno_t sss_authtok_set_password(struct sss_auth_token *tok,
> 101:errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
> 115:errno_t sss_authtok_set_ccfile(struct sss_auth_token *tok,
> 140:errno_t sss_authtok_set(struct sss_auth_token *tok,
> 154:errno_t sss_authtok_copy(struct sss_auth_token *src,
> 198:errno_t sss_authtok_set_2fa(struct sss_auth_token *tok,
> 222:errno_t sss_authtok_get_2fa(struct sss_auth_token *tok,
>
>
> and with your patch "errno_t" has to be defined before including
> authtok.h
> It's better to do not rely on the order of included header files.
Yes, you are right, thanks.
I suppose that correct solution would be then include just
'util/util_errors.h' in src/util/authtok.h. But I suppose we have
bigger fish to fry...or do you encourage me to send such a patch?
I had another
peak at the patch, I found out that we actually redefine
errno_t in 3 different header files (util_errors.h, sss_cli.h, nss_mc.h)
which is bad.
Attached patch removes already included header files...although I'm not
sure if it would not be better to actually get rid of as much nested
header files (header files included in header files) as possible and
rather include header files directly.
What do you think Lukas?
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
3:28 a.m.
On (21/05/15 17:26), Pavel Reichl wrote:
On 05/19/2015 02:25 PM, Pavel Reichl wrote:
>
>
>On 05/19/2015 02:15 PM, Lukas Slebodnik wrote:
>>On (19/05/15 13:59), Pavel Reichl wrote:
>>>Hello,
>>>
>>>please see attached simple patch. I believe that nested header files
>>>are
>>>sometimes necessary evil, but they should be avoided when possible. It
>>>makes
>>>difficult figuring out on which header files source file actually
>>>depend on
>>>and can increase compilation time.
>>>
>>>Thanks.
>>>From d11f9e4538cbb6d247f43e427347d59e1ca6c98f Mon Sep 17 00:00:00 2001
>>>From: Pavel Reichl <preichl(a)redhat.com>
>>>Date: Tue, 19 May 2015 07:35:11 -0400
>>>Subject: [PATCH] util: don't include util.h in src/util/authtok.h
>>>
>>>---
>>>src/util/authtok.c | 1 +
>>>src/util/authtok.h | 1 -
>>>2 files changed, 1 insertion(+), 1 deletion(-)
>>>
>>>diff --git a/src/util/authtok.c b/src/util/authtok.c
>>>index
45761df80175fded8a6c6e5dac8a90180b11d225..784512c0bd21128bd16d3663bf460a286e8b3eed
>>>100644
>>>--- a/src/util/authtok.c
>>>+++ b/src/util/authtok.c
>>>@@ -17,6 +17,7 @@
>>> along with this program. If not, see
>>><http://www.gnu.org/licenses/>.
>>>*/
>>>
>>>+#include "util/util.h"
>>>#include "authtok.h"
>>>
>>>struct sss_auth_token {
>>>diff --git a/src/util/authtok.h b/src/util/authtok.h
>>>index
cb366270832852281a222018f8e27feb1500ff01..d82168f5823e188e5c04fa418cb0ff52f6a52aac
>>>100644
>>>--- a/src/util/authtok.h
>>>+++ b/src/util/authtok.h
>>>@@ -20,7 +20,6 @@
>>>#ifndef __AUTHTOK_H__
>>>#define __AUTHTOK_H__
>>>
>>>-#include "util/util.h"
>>>#include "util/authtok-utils.h"
>>>#include "sss_client/sss_cli.h"
>>This is not right solution. Even thought everything compiles without any
>>problem.
>>
>>"util/util.h" was included because errnot_t is used in prototypes of
>>functions.
>>
>>sh$ grep -n errno_t src/util/authtok.h
>>70:errno_t sss_authtok_get_password(struct sss_auth_token *tok,
>>85:errno_t sss_authtok_set_password(struct sss_auth_token *tok,
>>101:errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
>>115:errno_t sss_authtok_set_ccfile(struct sss_auth_token *tok,
>>140:errno_t sss_authtok_set(struct sss_auth_token *tok,
>>154:errno_t sss_authtok_copy(struct sss_auth_token *src,
>>198:errno_t sss_authtok_set_2fa(struct sss_auth_token *tok,
>>222:errno_t sss_authtok_get_2fa(struct sss_auth_token *tok,
>>
>>
>>and with your patch "errno_t" has to be defined before including
>>authtok.h
>>It's better to do not rely on the order of included header files.
>Yes, you are right, thanks.
>
>I suppose that correct solution would be then include just
>'util/util_errors.h' in src/util/authtok.h. But I suppose we have bigger
>fish to fry...or do you encourage me to send such a patch?
I had another peak at the patch, I found out that we actually redefine
errno_t in 3 different header files (util_errors.h, sss_cli.h, nss_mc.h)
which is bad.
sss_cli.h should be included just in client code and should not be mixed
with responder/data_provider code. The same applies to nss_mc.h.
This is a reason why there are more typedefs.
Attached patch removes already included header files...although
I'm not sure
if it would not be better to actually get rid of as much nested header files
(header files included in header files) as possible and rather include header
files directly.
What do you think Lukas?
>>
>>LS
From 16bee39c2cc7a734abc5a19d6069c80eed2500cc Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 19 May 2015 07:35:11 -0400
Subject: [PATCH] util: remove needless header files
^^^^^^^^^^^^^^^
That's not true.
sss_cli.h is already included in authtok-utils.h, sysdb.h,
data_provider.h, responder.h, responder_packet.h
---
src/responder/sudo/sudosrv_cmd.c | 1 -
src/tests/cmocka/test_pam_srv.c | 1 -
src/tools/tools_mc_util.c | 1 -
src/util/authtok.c | 1 +
src/util/authtok.h | 2 --
5 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/responder/sudo/sudosrv_cmd.c b/src/responder/sudo/sudosrv_cmd.c
index dd636e949200dd49c1422a5789e9328dc4b25fb0..8d838babd0fbadcf9f1167cdeba4ec3855c246d5
100644
--- a/src/responder/sudo/sudosrv_cmd.c
+++ b/src/responder/sudo/sudosrv_cmd.c
@@ -27,7 +27,6 @@
#include "responder/common/responder_packet.h"
#include "responder/sudo/sudosrv_private.h"
#include "db/sysdb_sudo.h"
-#include "sss_client/sss_cli.h"
#include "responder/common/negcache.h"
sss_client/sss_cli.h defines SSS_GET_VERSION which is used in this file.
static errno_t sudosrv_cmd_send_reply(struct sudo_cmd_ctx *cmd_ctx,
diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
index 03faea249a31359d721e82cfd5bc2f9f375d0592..ac9da69e3610578ae267ce5c0d63a832fb0822bd
100644
--- a/src/tests/cmocka/test_pam_srv.c
+++ b/src/tests/cmocka/test_pam_srv.c
@@ -30,7 +30,6 @@
#include "responder/pam/pamsrv.h"
#include "responder/pam/pam_helpers.h"
#include "sss_client/pam_message.h"
-#include "sss_client/sss_cli.h"
#include "util/crypto/nss/nss_util.h"
SSS_PAM_AUTHENTICATE is defined sss_client/sss_cli.h which is used in this file
diff --git a/src/tools/tools_mc_util.c b/src/tools/tools_mc_util.c
index 9634b86e248a1937f68939c5a8d8a6301780a566..d54653abae26145a0aaa660b7ee3704b16f81a8d
100644
--- a/src/tools/tools_mc_util.c
+++ b/src/tools/tools_mc_util.c
@@ -26,7 +26,6 @@
#include "util/util.h"
#include "tools/tools_util.h"
#include "util/mmap_cache.h"
-#include "sss_client/sss_cli.h"
SSS_NSS_GETPWNAM is defined
sss_client/sss_cli.h which is used in this file
/* This is a copy of sss_mc_set_recycled present in
* src/responder/nss/nsssrv_mmap_cache.c. If you modify this function,
diff --git a/src/util/authtok.h b/src/util/authtok.h
index cb366270832852281a222018f8e27feb1500ff01..55f080e85806b7c241c6287a42a2cfa501a8ee19
100644
--- a/src/util/authtok.h
+++ b/src/util/authtok.h
@@ -20,9 +20,7 @@
#ifndef __AUTHTOK_H__
#define __AUTHTOK_H__
-#include "util/util.h"
#include "util/authtok-utils.h"
-#include "sss_client/sss_cli.h"
As I mentioned in previous mail; we *should not* rely on the order of header
files or included header fiels from other hedaer files.
In most cases we do not care about it (if code compiles without problem)
but on the other hand it's not a reason for micro-ptimalisation
to remove includes as in your patch. There still posibility that function
will be moved to another file(removed, ...) and the header file will need
to be included again.
All header files are guarded with defines. It's not a problem
when the same header files are included in header file and implementation
module. The only reason for removing lines with include will be if header file
is not used at all in module.
LS
5:08 a.m.
On 05/22/2015 10:28 AM, Lukas Slebodnik wrote:
On (21/05/15 17:26), Pavel Reichl wrote:
>
> On 05/19/2015 02:25 PM, Pavel Reichl wrote:
>>
>> On 05/19/2015 02:15 PM, Lukas Slebodnik wrote:
>>> On (19/05/15 13:59), Pavel Reichl wrote:
>>>> Hello,
>>>>
>>>> please see attached simple patch. I believe that nested header files
>>>> are
>>>> sometimes necessary evil, but they should be avoided when possible. It
>>>> makes
>>>> difficult figuring out on which header files source file actually
>>>> depend on
>>>> and can increase compilation time.
>>>>
>>>> Thanks.
>>> >From d11f9e4538cbb6d247f43e427347d59e1ca6c98f Mon Sep 17 00:00:00 2001
>>>> From: Pavel Reichl <preichl(a)redhat.com>
>>>> Date: Tue, 19 May 2015 07:35:11 -0400
>>>> Subject: [PATCH] util: don't include util.h in src/util/authtok.h
>>>>
>>>> ---
>>>> src/util/authtok.c | 1 +
>>>> src/util/authtok.h | 1 -
>>>> 2 files changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/util/authtok.c b/src/util/authtok.c
>>>> index
45761df80175fded8a6c6e5dac8a90180b11d225..784512c0bd21128bd16d3663bf460a286e8b3eed
>>>> 100644
>>>> --- a/src/util/authtok.c
>>>> +++ b/src/util/authtok.c
>>>> @@ -17,6 +17,7 @@
>>>> along with this program. If not, see
>>>> <http://www.gnu.org/licenses/>;.
>>>> */
>>>>
>>>> +#include "util/util.h"
>>>> #include "authtok.h"
>>>>
>>>> struct sss_auth_token {
>>>> diff --git a/src/util/authtok.h b/src/util/authtok.h
>>>> index
cb366270832852281a222018f8e27feb1500ff01..d82168f5823e188e5c04fa418cb0ff52f6a52aac
>>>> 100644
>>>> --- a/src/util/authtok.h
>>>> +++ b/src/util/authtok.h
>>>> @@ -20,7 +20,6 @@
>>>> #ifndef __AUTHTOK_H__
>>>> #define __AUTHTOK_H__
>>>>
>>>> -#include "util/util.h"
>>>> #include "util/authtok-utils.h"
>>>> #include "sss_client/sss_cli.h"
>>> This is not right solution. Even thought everything compiles without any
>>> problem.
>>>
>>> "util/util.h" was included because errnot_t is used in prototypes
of
>>> functions.
>>>
>>> sh$ grep -n errno_t src/util/authtok.h
>>> 70:errno_t sss_authtok_get_password(struct sss_auth_token *tok,
>>> 85:errno_t sss_authtok_set_password(struct sss_auth_token *tok,
>>> 101:errno_t sss_authtok_get_ccfile(struct sss_auth_token *tok,
>>> 115:errno_t sss_authtok_set_ccfile(struct sss_auth_token *tok,
>>> 140:errno_t sss_authtok_set(struct sss_auth_token *tok,
>>> 154:errno_t sss_authtok_copy(struct sss_auth_token *src,
>>> 198:errno_t sss_authtok_set_2fa(struct sss_auth_token *tok,
>>> 222:errno_t sss_authtok_get_2fa(struct sss_auth_token *tok,
>>>
>>>
>>> and with your patch "errno_t" has to be defined before including
>>> authtok.h
>>> It's better to do not rely on the order of included header files.
>> Yes, you are right, thanks.
>>
>> I suppose that correct solution would be then include just
>> 'util/util_errors.h' in src/util/authtok.h. But I suppose we have
bigger
>> fish to fry...or do you encourage me to send such a patch?
> I had another peak at the patch, I found out that we actually redefine
> errno_t in 3 different header files (util_errors.h, sss_cli.h, nss_mc.h)
> which is bad.
>
sss_cli.h should be included just in client code and should not be mixed
with responder/data_provider code. The same applies to nss_mc.h.
This is a reason why there are more typedefs.
> Attached patch removes already included header files...although I'm not sure
> if it would not be better to actually get rid of as much nested header files
> (header files included in header files) as possible and rather include header
> files directly.
>
> What do you think Lukas?
>>> LS
>From 16bee39c2cc7a734abc5a19d6069c80eed2500cc Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Tue, 19 May 2015 07:35:11 -0400
> Subject: [PATCH] util: remove needless header files
^^^^^^^^^^^^^^^
That's not true.
> sss_cli.h is already included in authtok-utils.h, sysdb.h,
> data_provider.h, responder.h, responder_packet.h
> ---
> src/responder/sudo/sudosrv_cmd.c | 1 -
> src/tests/cmocka/test_pam_srv.c | 1 -
> src/tools/tools_mc_util.c | 1 -
> src/util/authtok.c | 1 +
> src/util/authtok.h | 2 --
> 5 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/src/responder/sudo/sudosrv_cmd.c b/src/responder/sudo/sudosrv_cmd.c
> index
dd636e949200dd49c1422a5789e9328dc4b25fb0..8d838babd0fbadcf9f1167cdeba4ec3855c246d5 100644
> --- a/src/responder/sudo/sudosrv_cmd.c
> +++ b/src/responder/sudo/sudosrv_cmd.c
> @@ -27,7 +27,6 @@
> #include "responder/common/responder_packet.h"
> #include "responder/sudo/sudosrv_private.h"
> #include "db/sysdb_sudo.h"
> -#include "sss_client/sss_cli.h"
> #include "responder/common/negcache.h"
sss_client/sss_cli.h defines SSS_GET_VERSION which is used in this file.
> static errno_t sudosrv_cmd_send_reply(struct sudo_cmd_ctx *cmd_ctx,
> diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
> index
03faea249a31359d721e82cfd5bc2f9f375d0592..ac9da69e3610578ae267ce5c0d63a832fb0822bd 100644
> --- a/src/tests/cmocka/test_pam_srv.c
> +++ b/src/tests/cmocka/test_pam_srv.c
> @@ -30,7 +30,6 @@
> #include "responder/pam/pamsrv.h"
> #include "responder/pam/pam_helpers.h"
> #include "sss_client/pam_message.h"
> -#include "sss_client/sss_cli.h"
>
> #include "util/crypto/nss/nss_util.h"
>
SSS_PAM_AUTHENTICATE is defined sss_client/sss_cli.h which is used in this file
> diff --git a/src/tools/tools_mc_util.c b/src/tools/tools_mc_util.c
> index
9634b86e248a1937f68939c5a8d8a6301780a566..d54653abae26145a0aaa660b7ee3704b16f81a8d 100644
> --- a/src/tools/tools_mc_util.c
> +++ b/src/tools/tools_mc_util.c
> @@ -26,7 +26,6 @@
> #include "util/util.h"
> #include "tools/tools_util.h"
> #include "util/mmap_cache.h"
> -#include "sss_client/sss_cli.h"
SSS_NSS_GETPWNAM is defined sss_client/sss_cli.h which is used in this file
> /* This is a copy of sss_mc_set_recycled present in
> * src/responder/nss/nsssrv_mmap_cache.c. If you modify this function,
> diff --git a/src/util/authtok.h b/src/util/authtok.h
> index
cb366270832852281a222018f8e27feb1500ff01..55f080e85806b7c241c6287a42a2cfa501a8ee19 100644
> --- a/src/util/authtok.h
> +++ b/src/util/authtok.h
> @@ -20,9 +20,7 @@
> #ifndef __AUTHTOK_H__
> #define __AUTHTOK_H__
>
> -#include "util/util.h"
> #include "util/authtok-utils.h"
> -#include "sss_client/sss_cli.h"
As I mentioned in previous mail; we *should not* rely on the order of header
files or included header fiels from other hedaer files.
In most cases we do not care about it (if code compiles without problem)
but on the other hand it's not a reason for micro-ptimalisation
to remove includes as in your patch. There still posibility that function
will be moved to another file(removed, ...) and the header file will need
to be included again.
All header files are guarded with defines. It's not a problem
when the same header files are included in header file and implementation
module. The only reason for removing lines with include will be if header file
is not used at all in module.
LS
OK, I don't have strong preference here, so we can ignore the patch.
Thanks for the opinion.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
3255
days inactive
3258
days old
sssd-devel@lists.fedorahosted.org
5 comments
2 participants
participants (2)
-
Lukas Slebodnik -
Pavel Reichl