* 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).