log: split log variables from root object

The original plan was to avoid any global variables in the library. Thus
the logging variables were tied to the root object which is available
via the fabric API.

The default NVME API doesn't have the root object thus we had to add a
global variable to set log level etc. But the API to set these variables
is done via the root object.

This approach enforces the caller side to create an default root object
to initialize the default logging level. This introduces unnecessary
complexity on the caller side and wastes resources.

Let's split the default logging setting from the root object.

Signed-off-by: Daniel Wagner <dwagner@suse.de>

src/libnvme.map

@@ -5,6 +5,7 @@ LIBNVME_1.9 {
 		nvme_export_tls_key;
 		nvme_get_logging_level;
 		nvme_import_tls_key;
+		nvme_init_default_logging;
 		nvme_read_key;
 		nvme_scan_tls_keys;
 		nvme_submit_passthru64;

src/nvme/json.c

@@ -633,7 +633,7 @@ int json_dump_tree(nvme_root_t r)
 	}
 	json_object_object_add(json_root, "hosts", host_array);
 
-	ret = json_object_to_fd(fileno(r->fp), json_root, JSON_C_TO_STRING_PRETTY);
+	ret = json_object_to_fd(fileno(r->log.fp), json_root, JSON_C_TO_STRING_PRETTY);
 	if (ret < 0) {
 		nvme_msg(r, LOG_ERR, "Failed to write, %s\n",
 			 json_util_get_last_err());

src/nvme/log.c

@@ -26,13 +26,18 @@
 #define LOG_CLOCK CLOCK_MONOTONIC
 #endif
 
-static nvme_root_t root;
+static struct nvme_log def_log = {
+	.fp = NULL,
+	.level = DEFAULT_LOGLEVEL,
+	.pid = false,
+	.timestamp = false,
+};
 
 void __attribute__((format(printf, 4, 5)))
 __nvme_msg(nvme_root_t r, int level,
 	   const char *func, const char *format, ...)
 {
-	FILE *fp = stderr;
+	struct nvme_log *l;
 	va_list ap;
 	char pidbuf[16];
 	char timebuf[32];
@@ -50,18 +55,15 @@ __nvme_msg(nvme_root_t r, int level,
 	_cleanup_free_ char *message = NULL;
 	int idx = 0;
 
-	if (!r)
-		r = root;
-
 	if (r)
-		fp = r->fp;
+		l = &r->log;
+	else
+		l = &def_log;
 
-	if (r && lvl > r->log_level)
-		return;
-	if (!r && lvl > DEFAULT_LOGLEVEL)
+	if (level > l->level)
 		return;
 
-	if (r && r->log_timestamp) {
+	if (l->timestamp) {
 		struct timespec now;
 
 		clock_gettime(LOG_CLOCK, &now);
@@ -71,7 +73,7 @@ __nvme_msg(nvme_root_t r, int level,
 	} else
 		*timebuf = '\0';
 
-	if (r && r->log_pid) {
+	if (l->pid) {
 		snprintf(pidbuf, sizeof(pidbuf), "%ld", (long)getpid());
 		idx |= 1 << 1;
 	} else
@@ -89,42 +91,56 @@ __nvme_msg(nvme_root_t r, int level,
 		message = NULL;
 	va_end(ap);
 
-	fprintf(fp, "%s%s",
+	fprintf(l->fp, "%s%s",
 		header ? header : "<error>",
 		message ? message : "<error>");
 }
 
 void nvme_init_logging(nvme_root_t r, int lvl, bool log_pid, bool log_tstamp)
 {
-	r->log_level = lvl;
-	r->log_pid = log_pid;
-	r->log_timestamp = log_tstamp;
+	r->log.level = lvl;
+	r->log.pid = log_pid;
+	r->log.timestamp = log_tstamp;
 }
 
 int nvme_get_logging_level(nvme_root_t r, bool *log_pid, bool *log_tstamp)
 {
-	if (!r)
-		r = root;
-	if (!r)
-		return DEFAULT_LOGLEVEL;
+	struct nvme_log *l;
+
+	if (r)
+		l = &r->log;
+	else
+		l = &def_log;
+
 	if (log_pid)
-		*log_pid = r->log_pid;
+		*log_pid = l->pid;
 	if (log_tstamp)
-		*log_tstamp = r->log_timestamp;
-	return r->log_level;
+		*log_tstamp = l->timestamp;
+	return l->level;
+}
+
+void nvme_init_default_logging(FILE *fp, int level, bool log_pid, bool log_tstamp)
+{
+	def_log.fp = fp;
+	def_log.level = level;
+	def_log.pid = log_pid;
+	def_log.timestamp = log_tstamp;
 }
 
 void nvme_set_root(nvme_root_t r)
 {
-	root = r;
+	def_log.fp = r->log.fp;
+	def_log.level = r->log.level;
+	def_log.pid = r->log.pid;
+	def_log.timestamp = r->log.timestamp;
 }
 
 void nvme_set_debug(bool debug)
 {
-	root->log_level = debug ? LOG_DEBUG : DEFAULT_LOGLEVEL;
+	def_log.level = debug ? LOG_DEBUG : DEFAULT_LOGLEVEL;
 }
 
 bool nvme_get_debug(void)
 {
-	return root->log_level == LOG_DEBUG;
+	return def_log.level == LOG_DEBUG;
 }

src/nvme/log.h

@@ -35,6 +35,18 @@
  */
 void nvme_init_logging(nvme_root_t r, int lvl, bool log_pid, bool log_tstamp);
 
+/**
+ * nvme_init_default_logging() - Initialize default (fallback) logging
+ * @fp:		File descriptor for logging messages
+ * @lvl:	Logging level to set
+ * @log_pid:	Boolean to enable logging of the PID
+ * @log_tstamp:	Boolean to enable logging of the timestamp
+ *
+ * Sets the default logging settings for the library in case the root object
+ * is absent.
+ */
+void nvme_init_default_logging(FILE *fp, int lvl, bool log_pid, bool log_tstamp);
+
 /**
  * nvme_get_logging_level() - Get current logging level
  * @r:		nvme_root_t context
@@ -59,21 +71,24 @@ int nvme_get_logging_level(nvme_root_t r, bool *log_pid, bool *log_tstamp);
  * will be set as well. This means the global root object is always pointing to
  * the latest created root object. Note the first @nvme_free_tree call will reset
  * the global root object.
+ *
+ * This funciton is depricated. Use nvme_init_default_logging or/and
+ * nvme_init_logging instead.
  */
 void nvme_set_root(nvme_root_t r);
 
 /**
  * nvme_set_debug - Set NVMe command debugging output
  * @debug:	true to enable or false to disable
  *
- * Don't use it, it's debricated.
+ * This function is depricated. Use nvme_init_default_logging instead.
  */
 void nvme_set_debug(bool debug);
 
 /**
  * nvme_get_debug - Get NVMe command debugging output
  *
- * Don't use it, it's debricated.
+ * This function is depricated. Use nvme_get_logging_level instead.
  *
  * Return: false if disabled or true if enabled.
  */

src/nvme/mi.c

@@ -41,18 +41,26 @@ static bool nvme_mi_probe_enabled_default(void)
  */
 nvme_root_t nvme_mi_create_root(FILE *fp, int log_level)
 {
-	struct nvme_root *r = calloc(1, sizeof(*r));
+	struct nvme_root *r;
 
+	r = calloc(1, sizeof(*r));
 	if (!r) {
+		errno = ENOMEM;
 		return NULL;
 	}
-	r->log_level = log_level;
-	r->fp = stderr;
+
+	if (!fp)
+		fp = stderr;
+
+	r->log.fp = fp;
+	r->log.level = log_level;
+	nvme_init_default_logging(fp, log_level, false, false);
+
 	r->mi_probe_enabled = nvme_mi_probe_enabled_default();
-	if (fp)
-		r->fp = fp;
+
 	list_head_init(&r->hosts);
 	list_head_init(&r->endpoints);
+
 	return r;
 }
 

src/nvme/private.h

@@ -160,15 +160,19 @@ struct nvme_fabric_options {
 	bool trsvcid;
 };
 
+struct nvme_log {
+	FILE *fp;
+	int level;
+	bool pid;
+	bool timestamp;
+};
+
 struct nvme_root {
 	char *config_file;
 	char *application;
 	struct list_head hosts;
 	struct list_head endpoints; /* MI endpoints */
-	FILE *fp;
-	int log_level;
-	bool log_pid;
-	bool log_timestamp;
+	struct nvme_log log;
 	bool modified;
 	bool mi_probe_enabled;
 	struct nvme_fabric_options *options;

src/nvme/tree.c

@@ -187,19 +187,24 @@ int nvme_scan_topology(struct nvme_root *r, nvme_scan_filter_t f, void *f_args)
 
 nvme_root_t nvme_create_root(FILE *fp, int log_level)
 {
-	struct nvme_root *r = calloc(1, sizeof(*r));
+	struct nvme_root *r;
 
+	r = calloc(1, sizeof(*r));
 	if (!r) {
 		errno = ENOMEM;
 		return NULL;
 	}
-	r->log_level = log_level;
-	r->fp = stderr;
-	if (fp)
-		r->fp = fp;
+
+	if (!fp)
+		fp = stderr;
+
+	r->log.fp = fp;
+	r->log.level = log_level;
+	nvme_init_default_logging(fp, log_level, false, false);
+
 	list_head_init(&r->hosts);
 	list_head_init(&r->endpoints);
-	nvme_set_root(r);
+
 	return r;
 }
 
@@ -368,7 +373,6 @@ void nvme_free_tree(nvme_root_t r)
 		free(r->config_file);
 	if (r->application)
 		free(r->application);
-	nvme_set_root(NULL);
 	free(r);
 }