On (12/03/14 17:52), Pavel Reichl wrote: >On Mon, 2014-03-10 at 09:20 +0100, Lukas Slebodnik wrote: >> ehlo, >> >> We had in source code following pattern #elif HAVE_<name> >> It worked because undefined identifier(in some cases) was evaluated to 0. >> But we do not care about value of HAVE_SOMETHING. We just need to know >> whether identifier was defined. >> >> There is not equivalent to #ifdef (short for of #if definded) >> We need to use long form: #elif defined HAVE_<name> >> >> It causes also compiler warning with enabled compiler flag -Wundef. >> >> LS >> _______________________________________________ >> sssd-devel mailing list >> [email protected] >> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel > >Hello Lukas, > >Patch 1: >While testing I applied 3rd patch first and it (as expected) raised some >errors which were fixed by 2nd patch. >I can't see any problems which are addressed by this patch. There aren't >any #if directives or any other usage of (#define) constants from >config.h. Could you possibly describe in more detail the benefit of >this patch, please? In header file sss_utf8.h there are lines:
#ifdef HAVE_LIBUNISTRING #include <unistr.h> #include <unicase.h> #elif defined(HAVE_GLIB2) #include <glib.h> #endif But header file config.h was not included before header file sss_utf8.h in some places. So neither unistring header files nor glib header files were included. I realised these header files are necessary only in implementation module sss_utf8.c. I move part with ifdef into sss_utf8.c New patch is attached. > >Patch 2: >ACK > >Patch 3: >ACK - I think it's really useful - like it:-). > >Thanks. > >Pavel Reichl >
>From c164ac02f33cf4ad531cef30e856998e1f144ca7 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Fri, 14 Mar 2014 11:31:45 +0100 Subject: [PATCH 1/3] UTIL: Hide implementation details about unicode libraries. Header files from glib or unistring are only necessary in implementation module sss_utf8.c --- src/util/sss_utf8.c | 10 +++++++++- src/util/sss_utf8.h | 6 ------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/util/sss_utf8.c b/src/util/sss_utf8.c index 27c5cb60ad5db56a8d92a57bddacf28d7fab21ea..6952308268dd8829fd3532a10ed86dcb55fc5102 100644 --- a/src/util/sss_utf8.c +++ b/src/util/sss_utf8.c @@ -20,7 +20,15 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ -#include "util/util.h" +#include "config.h" + +#ifdef HAVE_LIBUNISTRING +#include <unistr.h> +#include <unicase.h> +#elif defined(HAVE_GLIB2) +#include <glib.h> +#endif + #include "sss_utf8.h" #ifdef HAVE_LIBUNISTRING diff --git a/src/util/sss_utf8.h b/src/util/sss_utf8.h index b7da76219c867b96193b414f1b9bdad75e995ec9..95b085f56a9b17c711029246d1fd0dedcade867b 100644 --- a/src/util/sss_utf8.h +++ b/src/util/sss_utf8.h @@ -23,12 +23,6 @@ #ifndef SSS_UTF8_H_ #define SSS_UTF8_H_ -#ifdef HAVE_LIBUNISTRING -#include <unistr.h> -#include <unicase.h> -#elif HAVE_GLIB2 -#include <glib.h> -#endif #include "util/util.h" #ifndef ENOMATCH -- 1.8.5.3
>From e5d0b3d46c41b58ebe6d01518bd5b9c48f7f3f00 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Mon, 9 Dec 2013 22:06:40 +0100 Subject: [PATCH 2/3] Use pattern #elif defined(identifier) We had in source code following pattern #elif HAVE_<name> It worked because undefined identifier(in some cases) was evaluated to 0. But we do not care about value of HAVE_SOMETHING. We just need to know whether identifier was defined. There is not equivalent to #ifdef (short for of #if definded) We need to use long form: #elif defined HAVE_<name> It causes also compiler warning with enabled compiler flag -Wundef. --- src/monitor/monitor_netlink.c | 4 ++-- src/sss_client/common.c | 4 ++-- src/util/sss_endian.h | 2 +- src/util/sss_ini.c | 22 +++++++++++----------- src/util/sss_krb5.h | 2 +- src/util/sss_utf8.c | 8 ++++---- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c index b4d636191c7f689a050e101320d234b248adb0d5..51e8ffe4725ac2943b39bf229022d2637b799cea 100644 --- a/src/monitor/monitor_netlink.c +++ b/src/monitor/monitor_netlink.c @@ -80,7 +80,7 @@ #define nlw_handle nl_sock -#elif HAVE_LIBNL1 +#elif defined(HAVE_LIBNL1) #define nlw_destroy_handle nl_handle_destroy #define nlw_alloc nl_handle_alloc @@ -413,7 +413,7 @@ static int nlw_enable_passcred(struct nlw_handle *nlp) { #ifdef HAVE_NL_SET_PASSCRED return nl_set_passcred(nlp, 1); /* 1 = enabled */ -#elif HAVE_NL_SOCKET_SET_PASSCRED +#elif defined(HAVE_NL_SOCKET_SET_PASSCRED) return nl_socket_set_passcred(nlp, 1); #else return EOK; /* not available in this version */ diff --git a/src/sss_client/common.c b/src/sss_client/common.c index 6044af072bbe5c580954fb917cef86b949b476d2..ebe783aba9cfa3dc11a529e7875966f139eea1af 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -1022,7 +1022,7 @@ static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr) { #ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST); -#elif HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP +#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP) return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP); #else #warning Robust mutexes are not supported on this platform. @@ -1034,7 +1034,7 @@ static int sss_mutex_consistent(pthread_mutex_t *mtx) { #ifdef HAVE_PTHREAD_MUTEX_CONSISTENT return pthread_mutex_consistent(mtx); -#elif HAVE_PTHREAD_MUTEX_CONSISTENT_NP +#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP) return pthread_mutex_consistent_np(mtx); #else #warning Robust mutexes are not supported on this platform. diff --git a/src/util/sss_endian.h b/src/util/sss_endian.h index 952f8e1aa63d60626eafc60a4a7c88965b735cd2..834c35980df39139b6e95df0b95675e183e75b4f 100644 --- a/src/util/sss_endian.h +++ b/src/util/sss_endian.h @@ -25,7 +25,7 @@ #ifdef HAVE_ENDIAN_H # include <endian.h> -#elif defined HAVE_SYS_ENDIAN_H +#elif defined(HAVE_SYS_ENDIAN_H) # include <sys/endian.h> #endif /* !HAVE_ENDIAN_H && !HAVE_SYS_ENDIAN_H */ diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index ce0d067551989b2fab920c0427f713ec9458c44a..5293ae93b9e733e7a4b7176531fd3f093852fd7d 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -34,7 +34,7 @@ #ifdef HAVE_LIBINI_CONFIG_V1 #include "ini_configobj.h" -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) #include "collection.h" #include "collection_tools.h" #else @@ -61,7 +61,7 @@ struct sss_ini_initdata { -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) struct sss_ini_initdata { struct collection_item *error_list; @@ -99,7 +99,7 @@ void sss_ini_close_file(struct sss_ini_initdata *init_data) ini_config_file_destroy(init_data->file); init_data->file = NULL; } -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) if (init_data->file != -1) { close(init_data->file); init_data->file = -1; @@ -118,7 +118,7 @@ int sss_ini_config_file_open(struct sss_ini_initdata *init_data, return ini_config_file_open(config_file, INI_META_STATS, &init_data->file); -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) return check_and_open_readonly(config_file, &init_data->file, 0, 0, (S_IRUSR|S_IWUSR), CHECK_REG); #endif @@ -139,7 +139,7 @@ int sss_ini_config_access_check(struct sss_ini_initdata *init_data) 0, /* owned by root */ (S_IRUSR|S_IWUSR), /* rw------- */ 0); /* check all there parts */ -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) return EOK; #endif } @@ -156,7 +156,7 @@ int sss_ini_get_stat(struct sss_ini_initdata *init_data) if (!init_data->cstat) return EIO; return EOK; -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) return fstat(init_data->file, &init_data->cstat); #endif @@ -173,7 +173,7 @@ int sss_ini_get_mtime(struct sss_ini_initdata *init_data, #ifdef HAVE_LIBINI_CONFIG_V1 return snprintf(timestr, timestr_len, "%llu", (long long unsigned)init_data->cstat->st_mtime); -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) return snprintf(timestr, timestr_len, "%llu", (long long unsigned)init_data->cstat.st_mtime); #endif @@ -247,7 +247,7 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, return ret; -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) /* Read the configuration into a collection */ ret = config_from_fd("sssd", @@ -308,7 +308,7 @@ int sss_ini_get_int_config_value(struct sss_ini_initdata *init_data, { #ifdef HAVE_LIBINI_CONFIG_V1 return ini_get_int_config_value(init_data->obj, strict, def, error); -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) return get_int_config_value(init_data->obj, strict, def, error); #endif } @@ -325,7 +325,7 @@ void sss_ini_config_destroy(struct sss_ini_initdata *init_data) ini_config_destroy(init_data->sssd_config); init_data->sssd_config = NULL; } -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) free_ini_config(init_data->sssd_config); #endif } @@ -355,7 +355,7 @@ int sss_confdb_create_ldif(TALLOC_CTX *mem_ctx, size_t attr_len; #ifdef HAVE_LIBINI_CONFIG_V1 struct value_obj *obj = NULL; -#elif HAVE_LIBINI_CONFIG_V0 +#elif defined(HAVE_LIBINI_CONFIG_V0) struct collection_item *obj = NULL; #endif diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h index 8838a9a6cb9dd12b0aa392d84e1da7eb1d1897f6..ee43f298381f7268dc8797830665e67f28146b4e 100644 --- a/src/util/sss_krb5.h +++ b/src/util/sss_krb5.h @@ -160,7 +160,7 @@ sss_krb5_free_keytab_entry_contents(krb5_context context, #ifdef HAVE_KRB5_TICKET_TIMES typedef krb5_ticket_times sss_krb5_ticket_times; -#elif HAVE_KRB5_TIMES +#elif defined(HAVE_KRB5_TIMES) typedef krb5_times sss_krb5_ticket_times; #endif diff --git a/src/util/sss_utf8.c b/src/util/sss_utf8.c index 6952308268dd8829fd3532a10ed86dcb55fc5102..7ba11ca759f3dddda6585dddb8add9a8395bba4b 100644 --- a/src/util/sss_utf8.c +++ b/src/util/sss_utf8.c @@ -36,7 +36,7 @@ void sss_utf8_free(void *ptr) { return free(ptr); } -#elif HAVE_GLIB2 +#elif defined(HAVE_GLIB2) void sss_utf8_free(void *ptr) { return g_free(ptr); @@ -57,7 +57,7 @@ uint8_t *sss_utf8_tolower(const uint8_t *s, size_t len, size_t *_nlen) if (_nlen) *_nlen = llen; return lower; } -#elif HAVE_GLIB2 +#elif defined(HAVE_GLIB2) uint8_t *sss_utf8_tolower(const uint8_t *s, size_t len, size_t *_nlen) { gchar *glower; @@ -94,7 +94,7 @@ bool sss_utf8_check(const uint8_t *s, size_t n) return false; } -#elif HAVE_GLIB2 +#elif defined(HAVE_GLIB2) bool sss_utf8_check(const uint8_t *s, size_t n) { return g_utf8_validate((const gchar *)s, n, NULL); @@ -141,7 +141,7 @@ errno_t sss_utf8_case_eq(const uint8_t *s1, const uint8_t *s2) return ENOMATCH; } -#elif HAVE_GLIB2 +#elif defined(HAVE_GLIB2) errno_t sss_utf8_case_eq(const uint8_t *s1, const uint8_t *s2) { gchar *gs1; -- 1.8.5.3
>From 039c58fbab781b587bafed166de81d61de8d5c07 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <[email protected]> Date: Mon, 9 Dec 2013 22:11:11 +0100 Subject: [PATCH 3/3] BUILD: Enable additional compiler warnings --- Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 750f626d17240a33e89a0b28c0ca5a6e2536c1cb..3193f1c1ab6bc671074f5d90f91ef5fa2c3b6d23 100644 --- a/Makefile.am +++ b/Makefile.am @@ -70,8 +70,8 @@ if WANT_AUX_INFO endif if HAVE_GCC AM_CFLAGS += -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith \ - -Wcast-qual -Wcast-align -Wwrite-strings \ - -Werror-implicit-function-declaration \ + -Wcast-qual -Wcast-align -Wwrite-strings -Wundef \ + -Werror-implicit-function-declaration -Winit-self \ -fno-strict-aliasing \ -std=gnu99 endif -- 1.8.5.3
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
