MINOR: tools: add minimal file name management

In proxies, stick-tables, servers, etc... at plenty of places we store
a file name and a line number. Some file names are the result of strdup()
(e.g. in proxies), others not (e.g. stick-tables) and leave dangling
pointers at the end of parsing. The risk of double-free is not null
either.

In order to stop this, let's first add a simple tool that allows to
register short strings inside a global list, these strings happening
to be server names. The strings are either duplicated and stored upon
failure to find them, or just added to this storage. Since file names
are not expected to disappear before the end of the process, for now
we don't even implement refcounting, and we free them all at the end.
There's already a drop_file_name() function to reset the pointer like
ha_free() used to do, and even if not strictly needed it's a good
habit to get used to doing it.

The strings are returned as const so that they're stored as-is in
structs, and that nasty free() calls are easily caught. The pointer
points to the char[] storage inside the node itself. This way later
if we want to implement refcounting, it will be trivial to just look
up a string and change its associated node's refcount. If needed,
comparisons can also be made on pointers.

For now they're not used yet and are released on deinit().

diff --git a/include/haproxy/tools-t.h b/include/haproxy/tools-t.h
index a63e0f6d20ecd1dca0f6918d9e8d550e152af2c8..ad881ae8f06f5b8fc139cc6f5d728f2e876f2a44 100644 (file)
--- a/include/haproxy/tools-t.h
+++ b/include/haproxy/tools-t.h
@@ -23,6 +23,7 @@
 #define _HAPROXY_TOOLS_T_H
 
 #include <netinet/in.h>
+#include <import/cebtree.h>
 
 /* size used for max length of decimal representation of long long int. */
 #define NB_LLMAX_STR (sizeof("-9223372036854775807")-1)
@@ -181,4 +182,16 @@ struct cbor_encode_ctx {
        void *e_fct_ctx;
 };
 
+/* An indexed file name node, to be used at various places where a config file
+ * location is expected. These elements live forever and are only released on
+ * deinit. The goal is to use them in place of a regular "char* file" in many
+ * structures so that they can remain referenced without being strduped nor
+ * refcounted. Refcounts might appear in the future. The root is file_names in
+ * tools.c.
+ */
+struct file_name_node {
+       struct ceb_node node; /* indexing node */
+       char name[VAR_ARRAY]; /* storage, used with cebus_*() */
+};
+
 #endif /* _HAPROXY_TOOLS_T_H */

diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h
index f4af49d513583abff1a1a659cac1238d89aec13c..2be18e34287d7b2640f7f1c51a29b42e02e59a20 100644 (file)
--- a/include/haproxy/tools.h
+++ b/include/haproxy/tools.h
@@ -96,6 +96,18 @@ static inline const char *ultoa(unsigned long n)
        return ultoa_r(n, itoa_str[0], sizeof(itoa_str[0]));
 }
 
+/* file names management */
+const char *copy_file_name(const char *name);
+void free_all_file_names();
+
+/* This is only used as a marker for call places where a free() of a file name
+ * is expected to be performed, and to reset the pointer.
+ */
+static inline void drop_file_name(const char **name)
+{
+       *name = NULL;
+}
+
 /*
  * unsigned long long ASCII representation
  *

diff --git a/src/haproxy.c b/src/haproxy.c
index 21cdcb34049bbd6ddd0013de5649ff80dd81cd2a..11fe16d8832f299e76a9e4216689aa4369abe34b 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2859,6 +2859,7 @@ void deinit(void)
        }
 
        vars_prune(&proc_vars, NULL, NULL);
+       free_all_file_names();
        pool_destroy_all();
        deinit_pollers();
 

diff --git a/src/tools.c b/src/tools.c
index 01189d3e0c200791974a6f0d373f8bc427fb11fc..49c273bb6ce3db393ecedbb454f1ea0575f75e96 100644 (file)
--- a/src/tools.c
+++ b/src/tools.c
@@ -57,6 +57,7 @@ extern void *__elf_aux_vector;
 #include <sys/prctl.h>
 #endif
 
+#include <import/cebus_tree.h>
 #include <import/eb32sctree.h>
 #include <import/eb32tree.h>
 #include <import/ebmbtree.h>
@@ -122,6 +123,12 @@ THREAD_LOCAL unsigned int statistical_prng_state = 2463534242U;
 /* set to true if this is a static build */
 int build_is_static = 0;
 
+/* known file names, made of file_name_node, to be used with file_name_*() */
+struct {
+       struct ceb_node *root; // file names tree, used with cebus_*()
+       __decl_thread(HA_RWLOCK_T lock);
+} file_names = { 0 };
+
 /* A global static table to store hashed words */
 static THREAD_LOCAL char hash_word[NB_L_HASH_WORD][20];
 static THREAD_LOCAL int index_hash = 0;
@@ -6886,6 +6893,71 @@ int restore_env(void)
        return 0;
 }
 
+/*
+ * File Name Lookups. Principle: the file_names struct at the top stores all
+ * known file names in a tree. Each node is a struct file_name_node. A take()
+ * call will either locate an existing entry or allocate a new one, and return
+ * a pointer to the string itself. The returned strings are const so as to
+ * easily detect unwanted free() calls. Structures using this mechanism only
+ * need a "const char *" and will never free their entries.
+ */
+
+/* finds or copies the file name, returns a reference to the char* storage area
+ * or NULL if name is NULL or upon allocation error.
+ */
+const char *copy_file_name(const char *name)
+{
+       struct file_name_node *file;
+       struct ceb_node *node;
+       size_t len;
+
+       if (!name)
+               return NULL;
+
+       HA_RWLOCK_RDLOCK(OTHER_LOCK, &file_names.lock);
+       node = cebus_lookup(&file_names.root, name);
+       HA_RWLOCK_RDUNLOCK(OTHER_LOCK, &file_names.lock);
+
+       if (node) {
+               file = container_of(node, struct file_name_node, node);
+               return file->name;
+       }
+
+       len = strlen(name);
+       file = malloc(sizeof(struct file_name_node) + len + 1);
+       if (!file)
+               return NULL;
+
+       memcpy(file->name, name, len + 1);
+       HA_RWLOCK_WRLOCK(OTHER_LOCK, &file_names.lock);
+       node = cebus_insert(&file_names.root, &file->node);
+       HA_RWLOCK_WRUNLOCK(OTHER_LOCK, &file_names.lock);
+
+       if (node != &file->node) {
+               /* the node was created in between */
+               free(file);
+               file = container_of(node, struct file_name_node, node);
+       }
+       return file->name;
+}
+
+/* free all registered file names */
+void free_all_file_names()
+{
+       struct file_name_node *file;
+       struct ceb_node *node;
+
+       HA_RWLOCK_WRLOCK(OTHER_LOCK, &file_names.lock);
+
+       while ((node = cebus_first(&file_names.root))) {
+               file = container_of(node, struct file_name_node, node);
+               cebus_delete(&file_names.root, node);
+               free(file);
+       }
+
+       HA_RWLOCK_WRUNLOCK(OTHER_LOCK, &file_names.lock);
+}
+
 /*
  * Local variables:
  *  c-indent-level: 8