[tarantool-patches] [PATCH] log: add missing assert into log_set_format()
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Mar 26 20:26:02 MSK 2019
On Mon, Mar 25, 2019 at 04:03:54PM +0300, Roman Khabibov wrote:
> commit 4bc90124d0cb27f8eec4a0e1d656fcbb35b919dc
> Author: Roman Khabibov <roman.habibov at tarantool.org>
> Date: Sat Mar 2 20:12:11 2019 +0300
>
> say: throw error when "json" used with syslog
>
> Add function returning logger type to throw error from lua
> when "json" used with syslog.
>
> Closes #3946
The commit message isn't very descriptive. Changed it to
say: fix assertion in log_format when called for boot or syslog logger
It's OK to use json format with the boot logger. As for syslog, let's
add a check to Lua's log_format() so that it fails gracefully rather
than raising an assertion.
Closes #3946
>
> diff --git a/extra/exports b/extra/exports
> index d72421123..21fa9bf00 100644
> --- a/extra/exports
> +++ b/extra/exports
> @@ -65,6 +65,7 @@ mp_encode_float
> mp_decode_double
> mp_decode_float
>
> +log_type
> say_set_log_level
> say_logrotate
> say_set_log_format
> diff --git a/src/lib/core/say.c b/src/lib/core/say.c
> index 50ee55692..aa3eb37d3 100644
> --- a/src/lib/core/say.c
> +++ b/src/lib/core/say.c
> @@ -161,6 +161,12 @@ level_to_syslog_priority(int level)
> }
> }
>
> +enum say_logger_type
> +log_type()
> +{
> + return log_default->type;
> +}
> +
> void
> log_set_level(struct log *log, enum say_level level)
> {
> @@ -170,7 +176,8 @@ log_set_level(struct log *log, enum say_level level)
> void
> log_set_format(struct log *log, log_format_func_t format_func)
> {
> - assert(format_func == say_format_plain ||
> + assert(format_func == say_format_json ||
> + format_func == say_format_plain ||
> log->type == SAY_LOGGER_STDERR ||
> log->type == SAY_LOGGER_PIPE || log->type == SAY_LOGGER_FILE);
This is a useless assertion now. Changed it to
assert(format_func == say_format_plain ||
log->type != SAY_LOGGER_SYSLOG);
>
> diff --git a/src/lib/core/say.h b/src/lib/core/say.h
> index 70050362c..d26c3ddef 100644
> --- a/src/lib/core/say.h
> +++ b/src/lib/core/say.h
> @@ -195,6 +195,13 @@ int
> log_say(struct log *log, int level, const char *filename,
> int line, const char *error, const char *format, ...);
>
> +/**
> + * Default logger type info.
> + * @retval say_logger_type.
> + */
> +enum say_logger_type
> +log_type();
> +
> /**
> * Set log level. Can be used dynamically.
> *
> diff --git a/src/lua/log.lua b/src/lua/log.lua
> index 0ac0e8f26..fe8f8921f 100644
> --- a/src/lua/log.lua
> +++ b/src/lua/log.lua
> @@ -4,10 +4,22 @@ local ffi = require('ffi')
> ffi.cdef[[
> typedef void (*sayfunc_t)(int level, const char *filename, int line,
> const char *error, const char *format, ...);
> +
> + enum say_logger_type {
> + SAY_LOGGER_BOOT,
> + SAY_LOGGER_STDERR,
> + SAY_LOGGER_FILE,
> + SAY_LOGGER_PIPE,
> + SAY_LOGGER_SYSLOG
> + };
> +
> + enum say_logger_type
> + log_type();
> +
> void
> say_set_log_level(int new_level);
>
> - void
> + int
Stray change. Removed it.
> say_set_log_format(enum say_format format);
>
>
> @@ -117,6 +129,9 @@ end
>
> local function log_format(format_name)
> if format_name == "json" then
> + if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
> + error("log_format: 'json' can't be used with syslog logger")
> + end
> ffi.C.say_set_log_format(ffi.C.SF_JSON)
> elseif format_name == "plain" then
> ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
> diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
> index 0c11702c8..f1f323699 100755
> --- a/test/app-tap/logger.test.lua
> +++ b/test/app-tap/logger.test.lua
> @@ -1,13 +1,14 @@
> #!/usr/bin/env tarantool
>
> local test = require('tap').test('log')
> -test:plan(24)
> +test:plan(26)
>
> --
> -- Check that Tarantool creates ADMIN session for #! script
> --
> local filename = "1.log"
> local message = "Hello, World!"
> +
> box.cfg{
> log=filename,
> memtx_memory=107374182,
> @@ -121,5 +122,10 @@ line = line:sub(1, index)
> message = json.decode(line)
> test:is(message.message, "log file has been reopened", "check message after log.rotate()")
> file:close()
> +
> +-- gh-3946: Check log_format usage.
> +test:ok(pcall(require('log').log_format, "json"), "can used with boot log")
> +test:ok(pcall(require('log').log_format, "plain"), "can used with boot log")
> +
This should go before box.cfg(), otherwise it tests nothing. Moved it.
Pushed to master and 2.1. Here's the final patch:
>From 6bcff2b5ffa48b96ff033cd1e0356c6dcba2eafc Mon Sep 17 00:00:00 2001
From: Roman Khabibov <roman.habibov at tarantool.org>
Date: Sat, 2 Mar 2019 20:12:11 +0300
Subject: [PATCH] say: fix assertion in log_format when called for boot or
syslog logger
It's OK to use json format with the boot logger. As for syslog, let's
add a check to Lua's log_format() so that it fails gracefully rather
than raising an assertion.
Closes #3946
diff --git a/extra/exports b/extra/exports
index d7242112..21fa9bf0 100644
--- a/extra/exports
+++ b/extra/exports
@@ -65,6 +65,7 @@ mp_encode_float
mp_decode_double
mp_decode_float
+log_type
say_set_log_level
say_logrotate
say_set_log_format
diff --git a/src/lib/core/say.c b/src/lib/core/say.c
index 50ee5569..68aa92f6 100644
--- a/src/lib/core/say.c
+++ b/src/lib/core/say.c
@@ -161,6 +161,12 @@ level_to_syslog_priority(int level)
}
}
+enum say_logger_type
+log_type()
+{
+ return log_default->type;
+}
+
void
log_set_level(struct log *log, enum say_level level)
{
@@ -171,9 +177,7 @@ void
log_set_format(struct log *log, log_format_func_t format_func)
{
assert(format_func == say_format_plain ||
- log->type == SAY_LOGGER_STDERR ||
- log->type == SAY_LOGGER_PIPE || log->type == SAY_LOGGER_FILE);
-
+ log->type != SAY_LOGGER_SYSLOG);
log->format_func = format_func;
}
diff --git a/src/lib/core/say.h b/src/lib/core/say.h
index 70050362..d26c3dde 100644
--- a/src/lib/core/say.h
+++ b/src/lib/core/say.h
@@ -196,6 +196,13 @@ log_say(struct log *log, int level, const char *filename,
int line, const char *error, const char *format, ...);
/**
+ * Default logger type info.
+ * @retval say_logger_type.
+ */
+enum say_logger_type
+log_type();
+
+/**
* Set log level. Can be used dynamically.
*
* @param log log object
diff --git a/src/lua/log.lua b/src/lua/log.lua
index 0ac0e8f2..312c14d5 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -4,6 +4,18 @@ local ffi = require('ffi')
ffi.cdef[[
typedef void (*sayfunc_t)(int level, const char *filename, int line,
const char *error, const char *format, ...);
+
+ enum say_logger_type {
+ SAY_LOGGER_BOOT,
+ SAY_LOGGER_STDERR,
+ SAY_LOGGER_FILE,
+ SAY_LOGGER_PIPE,
+ SAY_LOGGER_SYSLOG
+ };
+
+ enum say_logger_type
+ log_type();
+
void
say_set_log_level(int new_level);
@@ -117,6 +129,9 @@ end
local function log_format(format_name)
if format_name == "json" then
+ if ffi.C.log_type() == ffi.C.SAY_LOGGER_SYSLOG then
+ error("log_format: 'json' can't be used with syslog logger")
+ end
ffi.C.say_set_log_format(ffi.C.SF_JSON)
elseif format_name == "plain" then
ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
index 0c11702c..492d5ea0 100755
--- a/test/app-tap/logger.test.lua
+++ b/test/app-tap/logger.test.lua
@@ -3,6 +3,11 @@
local test = require('tap').test('log')
test:plan(24)
+-- gh-3946: Assertion failure when using log_format() before box.cfg()
+local log = require('log')
+log.log_format('json')
+log.log_format('plain')
+
--
-- Check that Tarantool creates ADMIN session for #! script
--
@@ -12,7 +17,6 @@ box.cfg{
log=filename,
memtx_memory=107374182,
}
-local log = require('log')
local fio = require('fio')
local json = require('json')
local fiber = require('fiber')
More information about the Tarantool-patches
mailing list