[RFC PATCH 0/9] -std=c99 / -std=c++0x -pedantic

[RFC PATCH 0/9] -std=c99 / -std=c++0x -pedantic

[Jani Nikula]

Hi all, this series was borne of curiousity about compiling the notmuch codebase
using -std=c99 / -std=c++0x -pedantic options. The C part is split into separate
patches to make it easier to see each warning and fix; with C++ I didn't bother
so much.

Turns out there are a few specific issues, but overall it's not too bad. However
my gut feeling is that some of the fixes to get standards compliance are uglier
than just using the GCC extensions. The question is, do we care about anything
other than GCC?

Comments and discussion welcome; that was the whole point here rather than any
serious effort for merging these. (But that can follow if people think this is
worth it.)

BR,
Jani.



Jani Nikula (9):
  build: use -std=c99 -pedantic for C source
  xutil: #define _POSIX_C_SOURCE to get strdup()
  lib: fix messages.c build warn
  lib: HACK: avoid warnings from talloc_steal()
  cli: fix warning about variadic macros
  cli: fix another warning about variadic macros
  util: fix warning about variadic macros
  lib: use -std=c++0x -pedantic
  test: smtp-dummy: fixes for -std=c99 -pedantic

 configure             |    4 ++--
 lib/database.cc       |    3 +--
 lib/message.cc        |    2 +-
 lib/messages.c        |    6 ++++--
 lib/notmuch-private.h |    8 +++++---
 lib/tags.c            |    5 ++++-
 lib/thread.cc         |    2 +-
 notmuch-client.h      |    9 ++++++---
 notmuch-setup.c       |    7 +++++--
 test/smtp-dummy.c     |    4 +++-
 util/error_util.h     |    8 +++++---
 util/xutil.c          |    2 ++
 12 files changed, 39 insertions(+), 21 deletions(-)

-- 
1.7.5.4

[RFC PATCH 1/9] build: use -std=c99 -pedantic for C source

[Jani Nikula]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 configure |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index e90b76f..f84262d 100755
--- a/configure
+++ b/configure
@@ -25,8 +25,8 @@ fi
 # environment variables)
 CC=${CC:-gcc}
 CXX=${CXX:-g++}
-CFLAGS=${CFLAGS:--O2}
-CXXFLAGS=${CXXFLAGS:-\$(CFLAGS)}
+CFLAGS=${CFLAGS:--O2 -std=c99 -pedantic}
+CXXFLAGS=${CXXFLAGS:--O2}
 LDFLAGS=${LDFLAGS:-}
 XAPIAN_CONFIG=${XAPIAN_CONFIG:-xapian-config}
 
-- 
1.7.5.4

[RFC PATCH 2/9] xutil: #define _POSIX_C_SOURCE to get strdup()

[Jani Nikula]

strdup() is not standard C99. #define _POSIX_C_SOURCE 200809L to use it.

This fixes -std=c99 -pedantic warning:

util/xutil.c: In function ‘xstrdup’:
util/xutil.c:74:5: warning: implicit declaration of function ‘strdup’ [-Wimplicit-function-declaration]
util/xutil.c:74:9: warning: assignment makes pointer from integer without a cast [enabled by default]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 util/xutil.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/util/xutil.c b/util/xutil.c
index ac496da..55b818b 100644
--- a/util/xutil.c
+++ b/util/xutil.c
@@ -18,6 +18,8 @@
  * Author: Carl Worth <cworth@cworth.org>
  */
 
+#define _POSIX_C_SOURCE 200809L /* for strdup() */
+
 #include <stdio.h>
 #include <string.h>
 
-- 
1.7.5.4

[RFC PATCH 3/9] lib: fix messages.c build warn

[Jani Nikula]

lib/messages.c: In function ‘notmuch_messages_move_to_next’:
lib/messages.c:131:2: warning: ISO C forbids ‘return’ with expression, in function returning void [-pedantic]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 lib/messages.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/messages.c b/lib/messages.c
index 7bcd1ab..1121864 100644
--- a/lib/messages.c
+++ b/lib/messages.c
@@ -127,8 +127,10 @@ notmuch_messages_get (notmuch_messages_t *messages)
 void
 notmuch_messages_move_to_next (notmuch_messages_t *messages)
 {
-    if (! messages->is_of_list_type)
-	return _notmuch_mset_messages_move_to_next (messages);
+    if (! messages->is_of_list_type) {
+	_notmuch_mset_messages_move_to_next (messages);
+	return;
+    }
 
     if (messages->iterator == NULL)
 	return;
-- 
1.7.5.4

Re: [RFC PATCH 3/9] lib: fix messages.c build warn

[Austin Clements]

I don't have much opinion on the other patches in this series (the C99
variadic macro stuff is unfortunate), but this one should go in.

Quoth Jani Nikula on Jan 08 at  1:26 am:
> lib/messages.c: In function ‘notmuch_messages_move_to_next’:
> lib/messages.c:131:2: warning: ISO C forbids ‘return’ with expression, in function returning void [-pedantic]
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  lib/messages.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/messages.c b/lib/messages.c
> index 7bcd1ab..1121864 100644
> --- a/lib/messages.c
> +++ b/lib/messages.c
> @@ -127,8 +127,10 @@ notmuch_messages_get (notmuch_messages_t *messages)
>  void
>  notmuch_messages_move_to_next (notmuch_messages_t *messages)
>  {
> -    if (! messages->is_of_list_type)
> -	return _notmuch_mset_messages_move_to_next (messages);
> +    if (! messages->is_of_list_type) {
> +	_notmuch_mset_messages_move_to_next (messages);
> +	return;
> +    }
>  
>      if (messages->iterator == NULL)
>  	return;

Re: [RFC PATCH 3/9] lib: fix messages.c build warn

[David Bremner]

On Sun,  8 Jan 2012 01:26:17 +0200, Jani Nikula <jani@nikula.org> wrote:
> lib/messages.c: In function ‘notmuch_messages_move_to_next’:
> lib/messages.c:131:2: warning: ISO C forbids ‘return’ with expression, in function returning void [-pedantic]

Pushed to master.

d

[RFC PATCH 4/9] lib: HACK: avoid warnings from talloc_steal()

[Jani Nikula]

talloc.h #defines talloc_steal() with macros to make it type safe, but
introduces a warning when compiled using -std=c99 -pedantic:

lib/tags.c: In function ‘_notmuch_tags_create’:
lib/tags.c:43:5: warning: ISO C forbids braced-groups within expressions [-pedantic]

Call _talloc_steal_loc() directly without the macro wrapper. We aren't even
interested in the return value the macro is so hard trying to provide type
safety for.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 lib/tags.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/lib/tags.c b/lib/tags.c
index c58924f..56c43e2 100644
--- a/lib/tags.c
+++ b/lib/tags.c
@@ -40,7 +40,10 @@ _notmuch_tags_create (const void *ctx, notmuch_string_list_t *list)
 	return NULL;
 
     tags->iterator = list->head;
-    talloc_steal (tags, list);
+
+    /* _talloc_steal_loc() is talloc_steal() without macro wrapping to avoid
+     * -std=c99 -pedantic warning. __location__ is defined in talloc.h. */
+    _talloc_steal_loc (tags, list, __location__);
 
     return tags;
 }
-- 
1.7.5.4

[RFC PATCH 5/9] cli: fix warning about variadic macros

[Jani Nikula]

notmuch-restore.c: In function ‘notmuch_restore_command’:
notmuch-restore.c:87:54: warning: ISO C99 requires rest arguments to be used [enabled by default]

and elsewhere. ##__VA_ARGS__ is a GCC CPP extension.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-client.h |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 517c010..3f4751a 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -93,14 +93,17 @@ typedef struct notmuch_show_params {
  *
  * Note that __location__ comes from talloc.h.
  */
-#define INTERNAL_ERROR(format, ...)			\
+
+#define _INTERNAL_ERROR(format, ...)			\
     do {						\
 	fprintf(stderr,					\
-		"Internal error: " format " (%s)\n",	\
-		##__VA_ARGS__, __location__);		\
+		"Internal error: " format "%s (%s)\n",	\
+		__VA_ARGS__, __location__);		\
 	exit (1);					\
     } while (0)
 
+#define INTERNAL_ERROR(...) _INTERNAL_ERROR(__VA_ARGS__, "")
+
 #define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
 
 #define STRNCMP_LITERAL(var, literal) \
-- 
1.7.5.4

[RFC PATCH 6/9] cli: fix another warning about variadic macros

[Jani Nikula]

notmuch-setup.c: In function ‘notmuch_setup_command’:
notmuch-setup.c:144:62: warning: ISO C99 requires rest arguments to be used [enabled by default]
notmuch-setup.c:174:18: warning: ISO C99 requires rest arguments to be used [enabled by default]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-setup.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/notmuch-setup.c b/notmuch-setup.c
index c3ea937..7b25680 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -102,9 +102,9 @@ notmuch_setup_command (unused (void *ctx),
     const char **new_tags;
     size_t new_tags_len;
 
-#define prompt(format, ...)					\
+#define __prompt(format, ...)					\
     do {							\
-	printf (format, ##__VA_ARGS__);				\
+	printf (format, __VA_ARGS__);				\
 	fflush (stdout);					\
 	if (getline (&response, &response_size, stdin) < 0) {	\
 	    printf ("Exiting.\n");				\
@@ -113,6 +113,9 @@ notmuch_setup_command (unused (void *ctx),
 	chomp_newline (response);				\
     } while (0)
 
+#define _prompt(format, ...) __prompt(format "%s", __VA_ARGS__)
+#define prompt(...) _prompt(__VA_ARGS__, "")
+
     config = notmuch_config_open (ctx, NULL, &is_new);
 
     if (is_new)
-- 
1.7.5.4

[RFC PATCH 7/9] util: fix warning about variadic macros

[Jani Nikula]

lib/string-list.c: In function ‘_notmuch_string_list_sort’:
lib/string-list.c:81:59: warning: ISO C99 requires rest arguments to be used [enabled by default]

lib/message-file.c: In function ‘notmuch_message_file_restrict_headersv’:
lib/message-file.c:147:90: warning: ISO C99 requires rest arguments to be used [enabled by default]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 util/error_util.h |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/util/error_util.h b/util/error_util.h
index bb15822..75709da 100644
--- a/util/error_util.h
+++ b/util/error_util.h
@@ -38,8 +38,10 @@ _internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2);
  *
  * Note that __location__ comes from talloc.h.
  */
-#define INTERNAL_ERROR(format, ...)			\
-    _internal_error (format " (%s).\n",			\
-		     ##__VA_ARGS__, __location__)
+#define _INTERNAL_ERROR(format, ...)			\
+    _internal_error (format "%s (%s).\n",		\
+		     __VA_ARGS__, __location__)
+
+#define INTERNAL_ERROR(...) _INTERNAL_ERROR(__VA_ARGS__, "")
 
 #endif
-- 
1.7.5.4

[RFC PATCH 8/9] lib: use -std=c++0x -pedantic

[Jani Nikula]

Introduces warnings such as:

In file included from /usr/include/gmime-2.4/gmime/gmime.h:39:0,
                 from lib/database.cc:31:
/usr/include/gmime-2.4/gmime/gmime-message.h:57:26: warning: comma at end of enumerator list [-pedantic]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 configure             |    2 +-
 lib/database.cc       |    3 +--
 lib/message.cc        |    2 +-
 lib/notmuch-private.h |    8 +++++---
 lib/thread.cc         |    2 +-
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index f84262d..30831f3 100755
--- a/configure
+++ b/configure
@@ -26,7 +26,7 @@ fi
 CC=${CC:-gcc}
 CXX=${CXX:-g++}
 CFLAGS=${CFLAGS:--O2 -std=c99 -pedantic}
-CXXFLAGS=${CXXFLAGS:--O2}
+CXXFLAGS=${CXXFLAGS:--O2 -std=c++0x -pedantic}
 LDFLAGS=${LDFLAGS:-}
 XAPIAN_CONFIG=${XAPIAN_CONFIG:-xapian-config}
 
diff --git a/lib/database.cc b/lib/database.cc
index 8103bd9..b59497b 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1352,8 +1352,7 @@ _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,
 	return status;
 
     if (message) {
-	*thread_id_ret = talloc_steal (ctx,
-				       notmuch_message_get_thread_id (message));
+	*thread_id_ret = (const char *) _talloc_steal_loc (ctx, notmuch_message_get_thread_id (message), __location__);
 
 	notmuch_message_destroy (message);
 
diff --git a/lib/message.cc b/lib/message.cc
index 0075425..ed7398a 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -220,7 +220,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
 									    message_id,
 									    &message);
     if (message)
-	return talloc_steal (notmuch, message);
+	return (notmuch_message_t *) _talloc_steal_loc (notmuch, message, __location__);
     else if (*status_ret)
 	return NULL;
 
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 60a932f..7694705 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -137,15 +137,17 @@ typedef enum _notmuch_private_status {
  * that the caller has previously handled any expected
  * notmuch_private_status_t values.)
  */
-#define COERCE_STATUS(private_status, format, ...)			\
+#define _COERCE_STATUS(private_status, format, ...)			\
     ((private_status >= (notmuch_private_status_t) NOTMUCH_STATUS_LAST_STATUS)\
      ?									\
-     (notmuch_status_t) _internal_error (format " (%s).\n",		\
-                                         ##__VA_ARGS__,			\
+     (notmuch_status_t) _internal_error (format "%s (%s).\n",		\
+                                         __VA_ARGS__,			\
                                          __location__)			\
      :									\
      (notmuch_status_t) private_status)
 
+#define COERCE_STATUS(private_status, ...) _COERCE_STATUS(private_status, __VA_ARGS__, "")
+
 typedef struct _notmuch_doc_id_set notmuch_doc_id_set_t;
 
 /* database.cc */
diff --git a/lib/thread.cc b/lib/thread.cc
index 0435ee6..e8f169c 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -224,7 +224,7 @@ _thread_add_message (notmuch_thread_t *thread,
     char *clean_author;
 
     _notmuch_message_list_add_message (thread->message_list,
-				       talloc_steal (thread, message));
+				       (notmuch_message_t *) _talloc_steal_loc (thread, message, __location__));
     thread->total_messages++;
 
     g_hash_table_insert (thread->message_hash,
-- 
1.7.5.4

[RFC PATCH 9/9] test: smtp-dummy: fixes for -std=c99 -pedantic

[Jani Nikula]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 test/smtp-dummy.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 3801a5e..1c29a7d 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -33,6 +33,8 @@
  * have been warned.
  */
 
+#define _POSIX_C_SOURCE 200809L /* for getline() and fdopen() */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -162,7 +164,7 @@ main (int argc, char *argv[])
 	memset (&addr, 0, sizeof (addr));
 	addr.sin_family = AF_INET;
 	addr.sin_port = htons (25025);
-	addr.sin_addr = *(struct in_addr *) hostinfo->h_addr;
+	addr.sin_addr = *(struct in_addr *) hostinfo->h_addr_list[0];
 	err = bind (sock, (struct sockaddr *) &addr, sizeof(addr));
 	if (err) {
 		fprintf (stderr, "Error: bind() failed: %s\n",
-- 
1.7.5.4