On Tue, 2016-01-19 at 02:54 +0100, Michal Židek wrote:
> On 01/19/2016 12:03 AM, Simo Sorce wrote:
> > Found this while working on another patch.
> >
> > It is not evident by this patch alone but ... "trust me" :-)
> > (I'll send the other patch next, try to apply just that one and see what
> > I mean if you want)
> >
> > Simo.
> >
> >
> 
> Hi Simo!
> 
> I wonder if including config.h indirectly through
> util.h is a good thing. It may be better
> to simply include config.h at the beginning of every
> .c file (after license) as a rule of thumb. This way
> even if we do not need util.h, we will have the same
> beginning of file and it will be more difficult to forget
> config.h. What do you think?

I a ok with such a rule, I can change the patch to explicitly add
config.h as first in all files I touched, are the other cleanups I did
also ok ?
Or do we want a patch that only adds config.h as the first header and
touches nothing else ?

> That being said, I know you made this patch in order
> to move work on another patch, so I definitely do not
> want to block the review.
> 
> CI passed:
> http://sssd-ci.duckdns.org/logs/job/35/72/summary.html
> 
> But I will wait with the ack until you respond to the
> question above.
> 
> Btw. we have quite a lot of files that do not use
> util.h that probably already use this rule (I did
> not check all of them, just did the grep). I think
> the rule would add consistency to the code.

Yes a lot of files do the "right" thing (between quotes because Lukas
apparently has a different opinion), just not all of them.

Simo.

> $ grep -r -L '#include .util/util.h.' ./src/ | grep '.*\.c$'
> ./src/tests/common_dbus.c
> ./src/tests/ad_ldap_opt-tests.c
> ./src/tests/find_uid-tests.c
> ./src/tests/common_tev.c
> ./src/tests/sbus_codegen_tests.c
> ./src/tests/common_check.c
> ./src/tests/sbus_tests.c
> ./src/tests/krb5_utils-tests.c
> ./src/tests/ipa_hbac-tests.c
> ./src/tests/simple_access-tests.c
> ./src/tests/ipa_ldap_opt-tests.c
> ./src/tests/sss_idmap-tests.c
> ./src/tests/responder_socket_access-tests.c
> ./src/tests/safe-format-tests.c
> ./src/tests/cmocka/test_nested_groups.c
> ./src/tests/cmocka/test_data_provider_be.c
> ./src/tests/cmocka/test_cert_utils.c
> ./src/tests/cmocka/test_ipa_subdomains_server.c
> ./src/tests/cmocka/test_find_uid.c
> ./src/tests/cmocka/test_copy_ccache.c
> ./src/tests/cmocka/test_sss_idmap.c
> ./src/tests/cmocka/test_resolv_fake.c
> ./src/tests/cmocka/test_ipa_idmap.c
> ./src/tests/cmocka/test_child_common.c
> ./src/tests/cmocka/test_expire_common.c
> ./src/tests/cmocka/test_authtok.c
> ./src/tests/cmocka/test_ipa_subdomains_utils.c
> ./src/tests/cmocka/test_fqnames.c
> ./src/tests/cmocka/test_copy_keytab.c
> ./src/tests/cmocka/test_utils.c
> ./src/tests/cmocka/test_ad_common.c
> ./src/tests/cmocka/test_fo_srv.c
> ./src/tests/cmocka/test_responder_cache_req.c
> ./src/tests/cmocka/common_mock_krb5.c
> ./src/tests/cmocka/test_ad_access_filter.c
> ./src/tests/cmocka/test_be_ptask.c
> ./src/tests/cmocka/test_ad_gpo.c
> ./src/tests/cmocka/test_search_bases.c
> ./src/tests/cmocka/test_sbus_opath.c
> ./src/tests/cmocka/test_pam_srv.c
> ./src/tests/cmocka/test_sysdb_subdomains.c
> ./src/tests/cmocka/test_sdap.c
> ./src/tests/cmocka/test_ldap_id_cleanup.c
> ./src/tests/cmocka/test_dp_opts.c
> ./src/tests/cmocka/test_responder_common.c
> ./src/tests/cmocka/test_nss_srv.c
> ./src/tests/cmocka/test_sysdb_views.c
> ./src/tests/cmocka/test_sss_sifp.c
> ./src/tests/cmocka/test_sdap_access.c
> ./src/tests/cmocka/test_ifp.c
> ./src/tests/cmocka/test_sysdb_utils.c
> ./src/tests/cmocka/test_dyndns.c
> ./src/tests/cmocka/test_ldap_auth.c
> ./src/tests/common_dom.c
> ./src/tests/dlopen-tests.c
> ./src/responder/pam/pam_helpers.c
> ./src/responder/ifp/ifpsrv_cmd.c
> ./src/responder/ifp/ifp_iface.c
> ./src/responder/ifp/ifpsrv_util.c
> ./src/responder/ifp/ifp_iface_nodes.c
> ./src/providers/data_provider_opts.c
> ./src/providers/proxy/proxy_id.c
> ./src/providers/proxy/proxy_init.c
> ./src/providers/proxy/proxy_auth.c
> ./src/providers/ipa/ipa_autofs.c
> ./src/providers/ipa/ipa_opts.c
> ./src/providers/ipa/ipa_subdomains.c
> ./src/providers/ipa/hbac_evaluator.c
> ./src/providers/ipa/ipa_hbac_common.c
> ./src/providers/ipa/ipa_selinux.c
> ./src/providers/ipa/ipa_common.c
> ./src/providers/ipa/ipa_subdomains_utils.c
> ./src/providers/ipa/ipa_init.c
> ./src/providers/ipa/ipa_selinux_maps.c
> ./src/providers/ipa/ipa_config.c
> ./src/providers/ipa/ipa_sudo.c
> ./src/providers/ipa/ipa_subdomains_server.c
> ./src/providers/ad/ad_autofs.c
> ./src/providers/ad/ad_sudo.c
> ./src/providers/ad/ad_opts.c
> ./src/providers/ad/ad_access.c
> ./src/providers/ad/ad_common.c
> ./src/providers/ad/ad_subdomains.c
> ./src/providers/dp_pam_data_util.c
> ./src/providers/krb5/krb5_common.c
> ./src/providers/krb5/krb5_wait_queue.c
> ./src/providers/krb5/krb5_init_shared.c
> ./src/providers/krb5/krb5_opts.c
> ./src/providers/krb5/krb5_init.c
> ./src/providers/dp_auth_util.c
> ./src/providers/data_provider_fo.c
> ./src/providers/data_provider_req.c
> ./src/providers/dp_sbus.c
> ./src/providers/simple/simple_access_check.c
> ./src/providers/simple/simple_access.c
> ./src/providers/ldap/sdap_async_sudo.c
> ./src/providers/ldap/sdap_sudo_cache.c
> ./src/providers/ldap/sdap_sudo.c
> ./src/providers/ldap/ldap_options.c
> ./src/providers/ldap/ldap_access.c
> ./src/providers/ldap/sdap_ad_groups.c
> ./src/providers/ldap/ldap_init.c
> ./src/providers/ldap/sdap_domain.c
> ./src/providers/ldap/ldap_opts.c
> ./src/providers/ldap/sdap_refresh.c
> ./src/providers/ldap/sdap_id_op.c
> ./src/providers/ldap/ldap_common.c
> ./src/providers/dp_refresh.c
> ./src/krb5_plugin/sssd_krb5_locator_plugin.c
> ./src/krb5_plugin/sssd_krb5_localauth_plugin.c
> ./src/sss_client/nss_netgroup.c
> ./src/sss_client/nss_mc_group.c
> ./src/sss_client/nss_passwd.c
> ./src/sss_client/nss_group.c
> ./src/sss_client/nss_mc_passwd.c
> ./src/sss_client/nfs/sss_nfs_client.c
> ./src/sss_client/autofs/sss_autofs.c
> ./src/sss_client/common.c
> ./src/sss_client/libwbclient/wbclient_common.c
> ./src/sss_client/libwbclient/wbc_idmap_sssd.c
> ./src/sss_client/libwbclient/wbc_guid.c
> ./src/sss_client/libwbclient/wbclient_sssd.c
> ./src/sss_client/libwbclient/wbc_sid_sssd.c
> ./src/sss_client/libwbclient/wbc_util_sssd.c
> ./src/sss_client/libwbclient/wbc_pwd_sssd.c
> ./src/sss_client/libwbclient/wbc_pam_sssd.c
> ./src/sss_client/libwbclient/wbc_sid_common.c
> ./src/sss_client/libwbclient/wbc_ctx_sssd.c
> ./src/sss_client/libwbclient/wbc_idmap_common.c
> ./src/sss_client/pam_sss.c
> ./src/sss_client/sudo_testcli/sudo_testcli.c
> ./src/sss_client/nss_mc_initgr.c
> ./src/sss_client/nss_mc_common.c
> ./src/sss_client/sudo/sss_sudo_response.c
> ./src/sss_client/nss_services.c
> ./src/sss_client/pam_message.c
> ./src/sss_client/pam_test_client.c
> ./src/sss_client/sssd_pac.c
> ./src/sss_client/idmap/sss_nss_idmap.c
> ./src/lib/sifp/sss_sifp_attrs.c
> ./src/lib/sifp/sss_sifp_parser.c
> ./src/lib/sifp/sss_sifp_utils.c
> ./src/lib/sifp/sss_sifp_common.c
> ./src/lib/sifp/sss_sifp_dbus.c
> ./src/lib/sifp/sss_sifp.c
> ./src/lib/cifs_idmap_sss/cifs_idmap_sss.c
> ./src/lib/idmap/sss_idmap.c
> ./src/util/atomic_io.c
> ./src/util/authtok.c
> ./src/util/cert/nss/cert.c
> ./src/util/sss_utf8.c
> ./src/util/io.c
> ./src/util/sss_config.c
> ./src/util/util_sss_idmap.c
> ./src/util/sss_selinux.c
> ./src/util/safe-format-string.c
> ./src/util/murmurhash3.c
> ./src/util/sss_python.c
> ./src/util/sss_tc_utf8.c
> ./src/util/authtok-utils.c
> ./src/db/sysdb_sudo.c
> ./src/db/sysdb_idmap.c
> ./src/db/sysdb_selinux.c
> ./src/db/sysdb_ssh.c
> ./src/db/sysdb_gpo.c
> ./src/db/sysdb_autofs.c
> ./src/tools/selinux.c
> ./src/tools/sss_signal.c
> ./src/python/pysss_murmur.c
> ./src/python/pysss_nss_idmap.c
> 
> 
> Michal
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


-- 
Simo Sorce * Red Hat, Inc * New York
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to