kgiusti commented on a change in pull request #1146:
URL: https://github.com/apache/qpid-dispatch/pull/1146#discussion_r622485407
##########
File path: src/log.c
##########
@@ -271,7 +280,7 @@ static const char* level_name(int level) {
static const char *SEPARATORS=", ;:";
/// Calculate the bit mask for a log enable string. Return -1 and set qd_error
on error.
-static int enable_mask(const char *enable_) {
+static int enable_mask(const char *enable_, bool log_error) {
Review comment:
This also appears to only be called with log_error=false, which makes
the log_error parameter & logic not necessary.
##########
File path: src/log.c
##########
@@ -525,71 +538,143 @@ void qd_log_finalize(void) {
log_sink_free_lh(DEQ_HEAD(sink_list));
}
-qd_error_t qd_log_entity(qd_entity_t *entity) {
-
+qd_error_t qd_log_entity(qd_entity_t *entity)
+{
qd_error_clear();
- //Obtain the log_source_lock global lock
- sys_mutex_lock(log_source_lock);
-
char* module = 0;
- char *output = 0;
+ char *outputFile = 0;
char *enable = 0;
+ int include_timestamp = 0;
+ int include_source = 0;
+
+ bool has_enable = false;
+ bool has_output_file = false;
+ bool has_include_timestamp = false;
+ bool has_include_source = false;
+ bool is_sink_syslog = false;
bool trace_enabled = false;
+ bool error_in_output = false;
+ bool error_in_enable = false;
do {
-
+ //
+ // A module attribute MUST be specified for a log entity.
+ // Every other attribute is optional.
+ //
module = qd_entity_get_string(entity, "module");
+
+ //
+ // If the module is not specified, there is nothing to do, just log an
+ // error and break out of this do loop.
+ //
QD_ERROR_BREAK();
Review comment:
IIRC these QD_ERROR_BREAK's force a jump to the end of the while (0)
loop.
At the end of the while (0) loop there is a mutex unlock call, which will be
unlocking a mutex which is not locked.
##########
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) {
Review comment:
IIUC this level_for_bit is only called by write_log(), which passes
log_error=false.
So why add the bool at all? Simply remove the qd_error since it will never
be called.
##########
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");
+}
+
/// Return NULL and set qd_error if not a valid level.
-static const level_t* level_for_name(const char *name, int len) {
+static const level_t* level_for_name(const char *name, int len, bool
log_error) {
Review comment:
same here, log_error is essentially unused.
--
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
