[SSSD] [PATCHES] Use pattern #elif defined(identifier)

Lukas Slebodnik lslebodn at redhat.com
Fri Mar 14 11:58:30 UTC 2014


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
>> sssd-devel at lists.fedorahosted.org
>> 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
>


-------------- next part --------------
>From c164ac02f33cf4ad531cef30e856998e1f144ca7 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
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

-------------- next part --------------
>From e5d0b3d46c41b58ebe6d01518bd5b9c48f7f3f00 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
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

-------------- next part --------------
>From 039c58fbab781b587bafed166de81d61de8d5c07 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
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



More information about the sssd-devel mailing list