unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* static variable cleanup
@ 2021-05-13 10:07 David Bremner
  2021-05-13 10:07 ` [PATCH 1/7] lib: make glib initialization thread-safe David Bremner
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch


Both the library and the CLI use a fair amount of static
variables. These are an obvious blocker for any kind of thread-safety,
even the limited goal of concurrently opening two databases for
reading in the same process.  This series cleans up some of the
low-hanging fruit.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/7] lib: make glib initialization thread-safe
  2021-05-13 10:07 static variable cleanup David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 10:07 ` [PATCH 2/7] lib/generate_thread_id: move static buffer to notmuch_database_t David Bremner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

In principle this could be done without depending on C++11 features,
but these features should be available since gcc 4.8.1, and this
localized usage is easy to replace if it turns out to be problematic
for portability.
---
 lib/Makefile.local    |  3 ++-
 lib/index.cc          | 43 ++++++++++++++++++++++---------------------
 lib/init.cc           | 21 +++++++++++++++++++++
 lib/message-file.c    |  6 +-----
 lib/notmuch-private.h |  7 +++++++
 lib/open.cc           | 24 +++---------------------
 6 files changed, 56 insertions(+), 48 deletions(-)
 create mode 100644 lib/init.cc

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 01cbb3f2..e2d4b91d 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -62,7 +62,8 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/thread-fp.cc     \
 	$(dir)/features.cc	\
 	$(dir)/prefix.cc	\
-	$(dir)/open.cc
+	$(dir)/open.cc		\
+	$(dir)/init.cc
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
 
diff --git a/lib/index.cc b/lib/index.cc
index 55c8372e..728bfb22 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -148,8 +148,6 @@ notmuch_filter_discard_non_term_class_init (NotmuchFilterDiscardNonTermClass *kl
     GObjectClass *object_class = G_OBJECT_CLASS (klass);
     GMimeFilterClass *filter_class = GMIME_FILTER_CLASS (klass);
 
-    parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
-
     object_class->finalize = notmuch_filter_discard_non_term_finalize;
 
     filter_class->copy = filter_copy;
@@ -240,30 +238,33 @@ filter_reset (GMimeFilter *gmime_filter)
  *
  * Returns: a new #NotmuchFilterDiscardNonTerm filter.
  **/
+static GType type = 0;
+
+static const GTypeInfo info = {
+    .class_size = sizeof (NotmuchFilterDiscardNonTermClass),
+    .base_init = NULL,
+    .base_finalize = NULL,
+    .class_init = (GClassInitFunc) notmuch_filter_discard_non_term_class_init,
+    .class_finalize = NULL,
+    .class_data = NULL,
+    .instance_size = sizeof (NotmuchFilterDiscardNonTerm),
+    .n_preallocs = 0,
+    .instance_init = NULL,
+    .value_table = NULL,
+};
+
+void
+_notmuch_filter_init () {
+    type = g_type_register_static (GMIME_TYPE_FILTER, "NotmuchFilterDiscardNonTerm", &info,
+				   (GTypeFlags) 0);
+    parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
+}
+
 static GMimeFilter *
 notmuch_filter_discard_non_term_new (GMimeContentType *content_type)
 {
-    static GType type = 0;
     NotmuchFilterDiscardNonTerm *filter;
 
-    if (! type) {
-	static const GTypeInfo info = {
-	    .class_size = sizeof (NotmuchFilterDiscardNonTermClass),
-	    .base_init = NULL,
-	    .base_finalize = NULL,
-	    .class_init = (GClassInitFunc) notmuch_filter_discard_non_term_class_init,
-	    .class_finalize = NULL,
-	    .class_data = NULL,
-	    .instance_size = sizeof (NotmuchFilterDiscardNonTerm),
-	    .n_preallocs = 0,
-	    .instance_init = NULL,
-	    .value_table = NULL,
-	};
-
-	type = g_type_register_static (GMIME_TYPE_FILTER, "NotmuchFilterDiscardNonTerm", &info,
-				       (GTypeFlags) 0);
-    }
-
     filter = (NotmuchFilterDiscardNonTerm *) g_object_new (type, NULL);
     filter->content_type = content_type;
     filter->state = 0;
diff --git a/lib/init.cc b/lib/init.cc
new file mode 100644
index 00000000..cf29200f
--- /dev/null
+++ b/lib/init.cc
@@ -0,0 +1,21 @@
+#include "notmuch-private.h"
+
+#include <mutex>
+
+static void do_init ()
+{
+    /* Initialize the GLib type system and threads */
+#if ! GLIB_CHECK_VERSION (2, 35, 1)
+    g_type_init ();
+#endif
+
+    g_mime_init ();
+    _notmuch_filter_init ();
+}
+
+void
+_notmuch_init ()
+{
+    static std::once_flag initialized;
+    std::call_once (initialized, do_init);
+}
diff --git a/lib/message-file.c b/lib/message-file.c
index 9e9b387f..647ccf3a 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -141,7 +141,6 @@ _notmuch_message_file_parse (notmuch_message_file_t *message)
 {
     GMimeParser *parser;
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
-    static int initialized = 0;
     bool is_mbox;
 
     if (message->message)
@@ -149,10 +148,7 @@ _notmuch_message_file_parse (notmuch_message_file_t *message)
 
     is_mbox = _is_mbox (message->stream);
 
-    if (! initialized) {
-	g_mime_init ();
-	initialized = 1;
-    }
+    _notmuch_init ();
 
     message->headers = g_hash_table_new_full (strcase_hash, strcase_equal,
 					      free, g_free);
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 10b1b024..093c29b1 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -469,11 +469,18 @@ _notmuch_database_link_message_to_parents (notmuch_database_t *notmuch,
 					   const char **thread_id);
 /* index.cc */
 
+void
+_notmuch_filter_init ();
+
 notmuch_status_t
 _notmuch_message_index_file (notmuch_message_t *message,
 			     notmuch_indexopts_t *indexopts,
 			     notmuch_message_file_t *message_file);
 
+/* init.cc */
+void
+_notmuch_init ();
+
 /* messages.c */
 
 typedef struct _notmuch_message_node {
diff --git a/lib/open.cc b/lib/open.cc
index 1e9c86fe..84b2d6b1 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -307,24 +307,6 @@ _set_database_path (notmuch_database_t *notmuch,
     _notmuch_config_cache (notmuch, NOTMUCH_CONFIG_DATABASE_PATH, path);
 }
 
-static void
-_init_libs ()
-{
-
-    static int initialized = 0;
-
-    /* Initialize the GLib type system and threads */
-#if ! GLIB_CHECK_VERSION (2, 35, 1)
-    g_type_init ();
-#endif
-
-    /* Initialize gmime */
-    if (! initialized) {
-	g_mime_init ();
-	initialized = 1;
-    }
-}
-
 static void
 _load_database_state (notmuch_database_t *notmuch)
 {
@@ -498,7 +480,7 @@ notmuch_database_open_with_config (const char *database_path,
     GKeyFile *key_file = NULL;
     bool split = false;
 
-    _init_libs ();
+    _notmuch_init ();
 
     notmuch = _alloc_notmuch ();
     if (! notmuch) {
@@ -595,7 +577,7 @@ notmuch_database_create_with_config (const char *database_path,
     int err;
     bool split = false;
 
-    _init_libs ();
+    _notmuch_init ();
 
     notmuch = _alloc_notmuch ();
     if (! notmuch) {
@@ -791,7 +773,7 @@ notmuch_database_load_config (const char *database_path,
     GKeyFile *key_file = NULL;
     bool split = false;
 
-    _init_libs ();
+    _notmuch_init ();
 
     notmuch = _alloc_notmuch ();
     if (! notmuch) {
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/7] lib/generate_thread_id: move static buffer to notmuch_database_t
  2021-05-13 10:07 static variable cleanup David Bremner
  2021-05-13 10:07 ` [PATCH 1/7] lib: make glib initialization thread-safe David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 10:07 ` [PATCH 3/7] lib/message: mark flag2tag as const David Bremner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Work towards the goal of concurrent access to different Xapian
databases from the same process.
---
 lib/add-message.cc     | 9 +++------
 lib/database-private.h | 4 ++++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/add-message.cc b/lib/add-message.cc
index 0c34d318..d4a00b17 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -40,17 +40,14 @@ parse_references (void *ctx,
 static const char *
 _notmuch_database_generate_thread_id (notmuch_database_t *notmuch)
 {
-    /* 16 bytes (+ terminator) for hexadecimal representation of
-     * a 64-bit integer. */
-    static char thread_id[17];
 
     notmuch->last_thread_id++;
 
-    sprintf (thread_id, "%016" PRIx64, notmuch->last_thread_id);
+    sprintf (notmuch->thread_id_str, "%016" PRIx64, notmuch->last_thread_id);
 
-    notmuch->writable_xapian_db->set_metadata ("last_thread_id", thread_id);
+    notmuch->writable_xapian_db->set_metadata ("last_thread_id", notmuch->thread_id_str);
 
-    return thread_id;
+    return notmuch->thread_id_str;
 }
 
 static char *
diff --git a/lib/database-private.h b/lib/database-private.h
index 0d12ec1e..1a73dacc 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -206,6 +206,10 @@ struct _notmuch_database {
     enum _notmuch_features features;
 
     unsigned int last_doc_id;
+
+    /* 16 bytes (+ terminator) for hexadecimal representation of
+     * a 64-bit integer. */
+    char thread_id_str[17];
     uint64_t last_thread_id;
 
     /* error reporting; this value persists only until the
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/7] lib/message: mark flag2tag as const
  2021-05-13 10:07 static variable cleanup David Bremner
  2021-05-13 10:07 ` [PATCH 1/7] lib: make glib initialization thread-safe David Bremner
  2021-05-13 10:07 ` [PATCH 2/7] lib/generate_thread_id: move static buffer to notmuch_database_t David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 10:07 ` [PATCH 4/7] CLI: centralize initialization in notmuch_client_init David Bremner
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This table is intended to be immutable
---
 lib/message.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/message.cc b/lib/message.cc
index 42d56acb..7af6ab82 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -68,7 +68,7 @@ struct maildir_flag_tag {
 };
 
 /* ASCII ordered table of Maildir flags and associated tags */
-static struct maildir_flag_tag flag2tag[] = {
+static const struct maildir_flag_tag flag2tag[] = {
     { 'D', "draft",   false },
     { 'F', "flagged", false },
     { 'P', "passed",  false },
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/7] CLI: centralize initialization in notmuch_client_init
  2021-05-13 10:07 static variable cleanup David Bremner
                   ` (2 preceding siblings ...)
  2021-05-13 10:07 ` [PATCH 3/7] lib/message: mark flag2tag as const David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 10:07 ` [PATCH 5/7] CLI/config: make immutable tables const David Bremner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Initially make sure gmime_filter_reply initialization is
thread-safe (assuming notmuch_client_init is only called once).
For tidyness, also put talloc initialization in the new function.
---
 Makefile.local       |  1 +
 gmime-filter-reply.c | 42 +++++++++++++++++++++---------------------
 gmime-filter-reply.h |  2 ++
 notmuch-client.h     |  4 ++++
 notmuch.c            |  7 +------
 5 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index bbb8f0b6..e12b94cd 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -231,6 +231,7 @@ notmuch_client_srcs =		\
 	gmime-filter-reply.c	\
 	hooks.c			\
 	notmuch.c		\
+	notmuch-client-init.c	\
 	notmuch-compact.c	\
 	notmuch-config.c	\
 	notmuch-count.c		\
diff --git a/gmime-filter-reply.c b/gmime-filter-reply.c
index 2b067669..35349cc8 100644
--- a/gmime-filter-reply.c
+++ b/gmime-filter-reply.c
@@ -43,29 +43,31 @@ static void filter_reset (GMimeFilter *filter);
 
 
 static GMimeFilterClass *parent_class = NULL;
+static GType type = 0;
+static const GTypeInfo info = {
+    .class_size = sizeof (GMimeFilterReplyClass),
+    .base_init = NULL,
+    .base_finalize = NULL,
+    .class_init = (GClassInitFunc) g_mime_filter_reply_class_init,
+    .class_finalize = NULL,
+    .class_data = NULL,
+    .instance_size = sizeof (GMimeFilterReply),
+    .n_preallocs = 0,
+    .instance_init = (GInstanceInitFunc) g_mime_filter_reply_init,
+    .value_table = NULL,
+};
+
+
+void
+g_mime_filter_reply_module_init (void)
+{
+    type = g_type_register_static (GMIME_TYPE_FILTER, "GMimeFilterReply", &info, (GTypeFlags) 0);
+    parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
+}
 
 GType
 g_mime_filter_reply_get_type (void)
 {
-    static GType type = 0;
-
-    if (! type) {
-	static const GTypeInfo info = {
-	    .class_size = sizeof (GMimeFilterReplyClass),
-	    .base_init = NULL,
-	    .base_finalize = NULL,
-	    .class_init = (GClassInitFunc) g_mime_filter_reply_class_init,
-	    .class_finalize = NULL,
-	    .class_data = NULL,
-	    .instance_size = sizeof (GMimeFilterReply),
-	    .n_preallocs = 0,
-	    .instance_init = (GInstanceInitFunc) g_mime_filter_reply_init,
-	    .value_table = NULL,
-	};
-
-	type = g_type_register_static (GMIME_TYPE_FILTER, "GMimeFilterReply", &info, (GTypeFlags) 0);
-    }
-
     return type;
 }
 
@@ -76,8 +78,6 @@ g_mime_filter_reply_class_init (GMimeFilterReplyClass *klass, unused (void *clas
     GObjectClass *object_class = G_OBJECT_CLASS (klass);
     GMimeFilterClass *filter_class = GMIME_FILTER_CLASS (klass);
 
-    parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
-
     object_class->finalize = g_mime_filter_reply_finalize;
 
     filter_class->copy = filter_copy;
diff --git a/gmime-filter-reply.h b/gmime-filter-reply.h
index 7cdefcd1..988fe2d6 100644
--- a/gmime-filter-reply.h
+++ b/gmime-filter-reply.h
@@ -21,6 +21,8 @@
 
 #include <gmime/gmime-filter.h>
 
+void g_mime_filter_reply_module_init (void);
+
 G_BEGIN_DECLS
 
 #define GMIME_TYPE_FILTER_REPLY            (g_mime_filter_reply_get_type ())
diff --git a/notmuch-client.h b/notmuch-client.h
index 270553ad..8227fea4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -250,6 +250,10 @@ json_quote_chararray (const void *ctx, const char *str, const size_t len);
 char *
 json_quote_str (const void *ctx, const char *str);
 
+/* notmuch-client-init.c */
+
+void notmuch_client_init (void);
+
 /* notmuch-config.c */
 
 typedef enum {
diff --git a/notmuch.c b/notmuch.c
index 2429999c..9ca3a4be 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -464,15 +464,10 @@ main (int argc, char *argv[])
 	{ }
     };
 
-    talloc_enable_null_tracking ();
+    notmuch_client_init ();
 
     local = talloc_new (NULL);
 
-    g_mime_init ();
-#if ! GLIB_CHECK_VERSION (2, 35, 1)
-    g_type_init ();
-#endif
-
     /* Globally default to the current output format version. */
     notmuch_format_version = NOTMUCH_FORMAT_CUR;
 
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/7] CLI/config: make immutable tables const
  2021-05-13 10:07 static variable cleanup David Bremner
                   ` (3 preceding siblings ...)
  2021-05-13 10:07 ` [PATCH 4/7] CLI: centralize initialization in notmuch_client_init David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 10:07 ` [PATCH 6/7] CLI: make static message strings const David Bremner
  2021-05-13 10:07 ` [PATCH 7/7] CLI/notmuch: make immutable tables const David Bremner
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Let the compiler help us catch bugs.
---
 notmuch-config.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index d9390c4d..3430a3d3 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -32,7 +32,7 @@ static const char toplevel_config_comment[] =
     "\n"
     " For more information about notmuch, see https://notmuchmail.org";
 
-struct config_group {
+static const struct config_group {
     const char *group_name;
     const char *comment;
 } group_comment_table [] = {
@@ -512,14 +512,14 @@ typedef struct config_key {
     bool (*validate)(const char *);
 } config_key_info_t;
 
-static struct config_key
+static const struct config_key
     config_key_table[] = {
     { "index.decrypt",   false,  NULL },
     { "index.header.",   true,   validate_field_name },
     { "query.",          true,   NULL },
 };
 
-static config_key_info_t *
+static const config_key_info_t *
 _config_key_info (const char *item)
 {
     for (size_t i = 0; i < ARRAY_SIZE (config_key_table); i++) {
@@ -583,7 +583,7 @@ notmuch_config_command_set (notmuch_database_t *notmuch,
 			    int argc, char *argv[])
 {
     char *group, *key;
-    config_key_info_t *key_info;
+    const config_key_info_t *key_info;
     notmuch_conffile_t *config;
     bool update_database = false;
     int opt_index, ret;
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/7] CLI: make static message strings const
  2021-05-13 10:07 static variable cleanup David Bremner
                   ` (4 preceding siblings ...)
  2021-05-13 10:07 ` [PATCH 5/7] CLI/config: make immutable tables const David Bremner
@ 2021-05-13 10:07 ` David Bremner
  2021-05-13 17:27   ` Tomi Ollila
  2021-05-13 10:07 ` [PATCH 7/7] CLI/notmuch: make immutable tables const David Bremner
  6 siblings, 1 reply; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This is both a bit clearer and avoids the possibility of modification.
---
 notmuch-insert.c | 2 +-
 notmuch-tag.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 00c00468..e3d87e4a 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -34,7 +34,7 @@ static volatile sig_atomic_t interrupted;
 static void
 handle_sigint (unused (int sig))
 {
-    static char msg[] = "Stopping...         \n";
+    static const char msg[] = "Stopping...         \n";
 
     /* This write is "opportunistic", so it's okay to ignore the
      * result.  It is not required for correctness, and if it does
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 667a75d6..de428c8e 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -27,7 +27,7 @@ static volatile sig_atomic_t interrupted;
 static void
 handle_sigint (unused (int sig))
 {
-    static char msg[] = "Stopping...         \n";
+    static const char msg[] = "Stopping...         \n";
 
     /* This write is "opportunistic", so it's okay to ignore the
      * result.  It is not required for correctness, and if it does
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/7] CLI/notmuch: make immutable tables const
  2021-05-13 10:07 static variable cleanup David Bremner
                   ` (5 preceding siblings ...)
  2021-05-13 10:07 ` [PATCH 6/7] CLI: make static message strings const David Bremner
@ 2021-05-13 10:07 ` David Bremner
  6 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2021-05-13 10:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Let the compiler enforce the immutability.
---
 notmuch.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/notmuch.c b/notmuch.c
index 9ca3a4be..d0a94fc2 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -139,7 +139,7 @@ notmuch_process_shared_indexing_options (notmuch_database_t *notmuch)
 }
 
 
-static command_t commands[] = {
+static const command_t commands[] = {
     { NULL, notmuch_command, NOTMUCH_COMMAND_CONFIG_CREATE | NOTMUCH_COMMAND_CONFIG_LOAD,
       "Notmuch main command." },
     { "setup", notmuch_setup_command, NOTMUCH_COMMAND_CONFIG_CREATE | NOTMUCH_COMMAND_CONFIG_LOAD,
@@ -189,7 +189,7 @@ typedef struct help_topic {
     const char *summary;
 } help_topic_t;
 
-static help_topic_t help_topics[] = {
+static const help_topic_t help_topics[] = {
     { "search-terms",
       "Common search term syntax." },
     { "hooks",
@@ -198,7 +198,7 @@ static help_topic_t help_topics[] = {
       "Message property conventions and documentation." },
 };
 
-static command_t *
+static const command_t *
 find_command (const char *name)
 {
     size_t i;
@@ -216,8 +216,8 @@ int notmuch_format_version;
 static void
 usage (FILE *out)
 {
-    command_t *command;
-    help_topic_t *topic;
+    const command_t *command;
+    const help_topic_t *topic;
     unsigned int i;
 
     fprintf (out,
@@ -308,8 +308,8 @@ exec_man (const char *page)
 static int
 _help_for (const char *topic_name)
 {
-    command_t *command;
-    help_topic_t *topic;
+    const command_t *command;
+    const help_topic_t *topic;
     unsigned int i;
 
     if (! topic_name) {
@@ -452,7 +452,7 @@ main (int argc, char *argv[])
     void *local;
     char *talloc_report;
     const char *command_name = NULL;
-    command_t *command;
+    const command_t *command;
     const char *config_file_name = NULL;
     notmuch_database_t *notmuch = NULL;
     int opt_index;
-- 
2.30.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/7] CLI: make static message strings const
  2021-05-13 10:07 ` [PATCH 6/7] CLI: make static message strings const David Bremner
@ 2021-05-13 17:27   ` Tomi Ollila
  2021-05-14 10:24     ` David Bremner
  0 siblings, 1 reply; 11+ messages in thread
From: Tomi Ollila @ 2021-05-13 17:27 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Thu, May 13 2021, David Bremner wrote:

> This is both a bit clearer and avoids the possibility of modification.
> ---
>  notmuch-insert.c | 2 +-
>  notmuch-tag.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 00c00468..e3d87e4a 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -34,7 +34,7 @@ static volatile sig_atomic_t interrupted;
>  static void
>  handle_sigint (unused (int sig))
>  {
> -    static char msg[] = "Stopping...         \n";
> +    static const char msg[] = "Stopping...         \n";

If compiler is smart enough, it can even save copy from read-only
area of the binary to read-write runtime memory area.

If not that smart it does the copy.

In my projects I've been using the following macro:
#define WriteCS(fd, str) write((fd), (str), sizeof(str) - 1)
but I don't know if that behaved any better (if we cared)...

But, in addition to these two identical copies of handle_sigint()
also notmuch-new.c and notmuch-reindex.c defines anoter 2 identical
copies of handle_sigint()...

I did not see that those two were also modified in this series...

Other that these, the series looked ok...

Tomi

>  
>      /* This write is "opportunistic", so it's okay to ignore the
>       * result.  It is not required for correctness, and if it does
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 667a75d6..de428c8e 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -27,7 +27,7 @@ static volatile sig_atomic_t interrupted;
>  static void
>  handle_sigint (unused (int sig))
>  {
> -    static char msg[] = "Stopping...         \n";
> +    static const char msg[] = "Stopping...         \n";
>  
>      /* This write is "opportunistic", so it's okay to ignore the
>       * result.  It is not required for correctness, and if it does
> -- 
> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/7] CLI: make static message strings const
  2021-05-13 17:27   ` Tomi Ollila
@ 2021-05-14 10:24     ` David Bremner
  2021-05-14 14:39       ` Tomi Ollila
  0 siblings, 1 reply; 11+ messages in thread
From: David Bremner @ 2021-05-14 10:24 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> In my projects I've been using the following macro:
> #define WriteCS(fd, str) write((fd), (str), sizeof(str) - 1)
> but I don't know if that behaved any better (if we cared)...

I'm not sure. The sizeof here is actually slightly treacherous (iiuc,
making things const char * will blow up the world) so I prefer it out in
the open.

> But, in addition to these two identical copies of handle_sigint()
> also notmuch-new.c and notmuch-reindex.c defines anoter 2 identical
> copies of handle_sigint()...
>
> I did not see that those two were also modified in this series...

Good catch! I did the same for those two and pushed the modified series.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/7] CLI: make static message strings const
  2021-05-14 10:24     ` David Bremner
@ 2021-05-14 14:39       ` Tomi Ollila
  0 siblings, 0 replies; 11+ messages in thread
From: Tomi Ollila @ 2021-05-14 14:39 UTC (permalink / raw)
  To: David Bremner, notmuch

On Fri, May 14 2021, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> In my projects I've been using the following macro:
>> #define WriteCS(fd, str) write((fd), (str), sizeof(str) - 1)
>> but I don't know if that behaved any better (if we cared)...
>
> I'm not sure. The sizeof here is actually slightly treacherous (iiuc,
> making things const char * will blow up the world) so I prefer it out in
> the open.

Right, const char * would make sizeof 8 (or 4).

(I just fixed bug somewhere where first 4 chars of strings were
compared for equality ;/).

The WriteCS meant (Const String), perhaps LS (Literal String) would be 
better. (and write((fd), "" str "", sizeof(str) - 1)).

... have to check if this works, and start using if it works as expected!

Tomi

>
>> But, in addition to these two identical copies of handle_sigint()
>> also notmuch-new.c and notmuch-reindex.c defines anoter 2 identical
>> copies of handle_sigint()...
>>
>> I did not see that those two were also modified in this series...
>
> Good catch! I did the same for those two and pushed the modified series.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-05-14 14:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-13 10:07 static variable cleanup David Bremner
2021-05-13 10:07 ` [PATCH 1/7] lib: make glib initialization thread-safe David Bremner
2021-05-13 10:07 ` [PATCH 2/7] lib/generate_thread_id: move static buffer to notmuch_database_t David Bremner
2021-05-13 10:07 ` [PATCH 3/7] lib/message: mark flag2tag as const David Bremner
2021-05-13 10:07 ` [PATCH 4/7] CLI: centralize initialization in notmuch_client_init David Bremner
2021-05-13 10:07 ` [PATCH 5/7] CLI/config: make immutable tables const David Bremner
2021-05-13 10:07 ` [PATCH 6/7] CLI: make static message strings const David Bremner
2021-05-13 17:27   ` Tomi Ollila
2021-05-14 10:24     ` David Bremner
2021-05-14 14:39       ` Tomi Ollila
2021-05-13 10:07 ` [PATCH 7/7] CLI/notmuch: make immutable tables const David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).