ChugR commented on a change in pull request #1146:
URL: https://github.com/apache/qpid-dispatch/pull/1146#discussion_r622470098



##########
File path: src/log.c
##########
@@ -222,27 +222,36 @@ static level_t levels[] = {
     LEVEL("critical", QD_LOG_CRITICAL, LOG_CRIT)
 };
 
+static const level_t invalid_level = {"invalid", -2, -2, 0};
+
 static char level_names[TEXT_MAX] = {0}; /* Set up in qd_log_initialize */
 
 /// Return NULL and set qd_error if not a valid bit.
-static const level_t* level_for_bit(int bit) {
+static const level_t* level_for_bit(int bit, bool log_error) {
     level_index_t i = 0;
     while (i < N_LEVELS && levels[i].bit != bit) ++i;
     if (i == N_LEVELS) {
-        qd_error(QD_ERROR_CONFIG, "'%d' is not a valid log level bit.", bit);
-        return NULL;
+        if (log_error)
+            qd_error(QD_ERROR_CONFIG, "'%d' is not a valid log level bit.", 
bit);
+        return &invalid_level;
     }
     return &levels[i];
 }
 
+static bool is_log_level_invalid(const level_t *level)
+{
+    return ! strcmp(level->name, "invalid");

Review comment:
       A better (faster) comparison might be 
       level->bit != -2 
   Then inline it.

##########
File path: src/log.c
##########
@@ -425,7 +434,6 @@ void qd_vlog_impl(qd_log_source_t *source, qd_log_level_t 
level, bool check_leve
     }
 
     // Bounded buffer of log entries, keep most recent.
-    sys_mutex_lock(log_lock);
     qd_log_entry_t *entry = new_qd_log_entry_t();
     DEQ_ITEM_INIT(entry);
     entry->module = source->module;

Review comment:
       source->module is a char*. Copying the pointer to entry leaves it 
vulnerable to deletion as described in the comment at line 446. This string 
should be copied into entry the same way that entry->file is strdup'd on line 
441. Then the entry is not depending on source still existing the whole time 
entry is in the entries list (line 542).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to