From 12ca818ffddb77eb6a0fabe369a5bcbf6994ff8b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 Sep 2015 18:22:42 +0200 Subject: tree-wide: clean up log_syntax() usage - Rely everywhere that we use abs() on the error code passed in anyway, thus don't need to explicitly negate what we pass in - Never attach synthetic error number information to log messages. Only log about errors we *receive* with the error number we got there, don't log any synthetic error, that don#t even propagate, but just eat up. - Be more careful with attaching exactly the error we get, instead of errno or unrelated errors randomly. - Fix one occasion where the error number and line number got swapped. - Make sure we never tape over OOM issues, or inability to resolve specifiers --- src/shared/conf-parser.c | 47 +++++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 26 deletions(-) (limited to 'src/shared/conf-parser.c') diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 946eac6823..ed8a926a13 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -147,8 +147,7 @@ static int next_assignment(const char *unit, /* Warn about unknown non-extension fields. */ if (!relaxed && !startswith(lvalue, "X-")) - log_syntax(unit, LOG_WARNING, filename, line, EINVAL, - "Unknown lvalue '%s' in section '%s'", lvalue, section); + log_syntax(unit, LOG_WARNING, filename, line, 0, "Unknown lvalue '%s' in section '%s'", lvalue, section); return 0; } @@ -196,8 +195,7 @@ static int parse_line(const char* unit, * Support for them should be eventually removed. */ if (!allow_include) { - log_syntax(unit, LOG_ERR, filename, line, EBADMSG, - ".include not allowed here. Ignoring."); + log_syntax(unit, LOG_ERR, filename, line, 0, ".include not allowed here. Ignoring."); return 0; } @@ -216,8 +214,7 @@ static int parse_line(const char* unit, assert(k > 0); if (l[k-1] != ']') { - log_syntax(unit, LOG_ERR, filename, line, EBADMSG, - "Invalid section header '%s'", l); + log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid section header '%s'", l); return -EBADMSG; } @@ -228,8 +225,7 @@ static int parse_line(const char* unit, if (sections && !nulstr_contains(sections, n)) { if (!relaxed && !startswith(n, "X-")) - log_syntax(unit, LOG_WARNING, filename, line, EINVAL, - "Unknown section '%s'. Ignoring.", n); + log_syntax(unit, LOG_WARNING, filename, line, 0, "Unknown section '%s'. Ignoring.", n); free(n); *section = mfree(*section); @@ -248,16 +244,15 @@ static int parse_line(const char* unit, if (sections && !*section) { if (!relaxed && !*section_ignored) - log_syntax(unit, LOG_WARNING, filename, line, EINVAL, - "Assignment outside of section. Ignoring."); + log_syntax(unit, LOG_WARNING, filename, line, 0, "Assignment outside of section. Ignoring."); return 0; } e = strchr(l, '='); if (!e) { - log_syntax(unit, LOG_WARNING, filename, line, EINVAL, "Missing '='."); - return -EBADMSG; + log_syntax(unit, LOG_WARNING, filename, line, 0, "Missing '='."); + return -EINVAL; } *e = 0; @@ -441,7 +436,7 @@ int config_parse_many(const char *conf_file, \ r = conv_func(rvalue, i); \ if (r < 0) \ - log_syntax(unit, LOG_ERR, filename, line, -r, \ + log_syntax(unit, LOG_ERR, filename, line, r, \ "Failed to parse %s value, ignoring: %s", \ #type, rvalue); \ \ @@ -479,7 +474,7 @@ int config_parse_iec_size(const char* unit, r = parse_size(rvalue, 1024, &v); if (r < 0 || (uint64_t) (size_t) v != v) { - log_syntax(unit, LOG_ERR, filename, line, r < 0 ? r : ERANGE, "Failed to parse size value, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse size value, ignoring: %s", rvalue); return 0; } @@ -509,7 +504,7 @@ int config_parse_si_size(const char* unit, r = parse_size(rvalue, 1000, &v); if (r < 0 || (uint64_t) (size_t) v != v) { - log_syntax(unit, LOG_ERR, filename, line, r < 0 ? r : ERANGE, "Failed to parse size value, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse size value, ignoring: %s", rvalue); return 0; } @@ -564,8 +559,7 @@ int config_parse_bool(const char* unit, k = parse_boolean(rvalue); if (k < 0) { - log_syntax(unit, LOG_ERR, filename, line, -k, - "Failed to parse boolean value, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, k, "Failed to parse boolean value, ignoring: %s", rvalue); return 0; } @@ -669,7 +663,7 @@ int config_parse_path( } if (!path_is_absolute(rvalue)) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Not an absolute path, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, "Not an absolute path, ignoring: %s", rvalue); return 0; } @@ -740,8 +734,7 @@ int config_parse_strv(const char *unit, return log_oom(); } if (!isempty(state)) - log_syntax(unit, LOG_ERR, filename, line, EINVAL, - "Trailing garbage, ignoring."); + log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring."); return 0; } @@ -759,14 +752,16 @@ int config_parse_mode( void *userdata) { mode_t *m = data; + int r; assert(filename); assert(lvalue); assert(rvalue); assert(data); - if (parse_mode(rvalue, m) < 0) { - log_syntax(unit, LOG_ERR, filename, line, errno, "Failed to parse mode value, ignoring: %s", rvalue); + r = parse_mode(rvalue, m); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse mode value, ignoring: %s", rvalue); return 0; } @@ -795,7 +790,7 @@ int config_parse_log_facility( x = log_facility_unshifted_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Failed to parse log facility, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse log facility, ignoring: %s", rvalue); return 0; } @@ -826,7 +821,7 @@ int config_parse_log_level( x = log_level_from_string(rvalue); if (x < 0) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Failed to parse log level, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse log level, ignoring: %s", rvalue); return 0; } @@ -855,7 +850,7 @@ int config_parse_signal( r = signal_from_string_try_harder(rvalue); if (r <= 0) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Failed to parse signal name, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse signal name, ignoring: %s", rvalue); return 0; } @@ -884,7 +879,7 @@ int config_parse_personality( p = personality_from_string(rvalue); if (p == PERSONALITY_INVALID) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Failed to parse personality, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse personality, ignoring: %s", rvalue); return 0; } -- cgit v1.2.3-54-g00ecf From 0e05ee044a6e23745bb8906ad91ec7b97c37dbac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 Sep 2015 20:07:24 +0200 Subject: log: move log_invalid_utf8() to log.h Also, make sure it follows the same scheme as log_syntax() does in its behaviour. --- src/basic/log.h | 12 ++++++++++++ src/core/load-fragment.c | 8 ++++---- src/shared/conf-parser.c | 6 +++--- src/shared/conf-parser.h | 7 ------- 4 files changed, 19 insertions(+), 14 deletions(-) (limited to 'src/shared/conf-parser.c') diff --git a/src/basic/log.h b/src/basic/log.h index 569762d083..369d6b1127 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -227,3 +227,15 @@ int log_syntax_internal( ? log_syntax_internal(unit, _level, config_file, config_line, _e, __FILE__, __LINE__, __func__, __VA_ARGS__) \ : -abs(_e); \ }) + +#define log_syntax_invalid_utf8(unit, level, config_file, config_line, rvalue) \ + ({ \ + int _level = (level); \ + if (log_get_max_level() >= LOG_PRI(_level)) { \ + _cleanup_free_ char *_p = NULL; \ + _p = utf8_escape_invalid(rvalue); \ + log_syntax_internal(unit, _level, config_file, config_line, 0, __FILE__, __LINE__, __func__, \ + "String is not UTF-8 clean, ignoring assignment: %s", strna(_p)); \ + } \ + -EINVAL; \ + }) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 7180252f36..7d6d46b904 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -259,7 +259,7 @@ int config_parse_unit_path_strv_printf( } if (!utf8_is_valid(k)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); return 0; } @@ -1835,7 +1835,7 @@ int config_parse_working_directory( path_kill_slashes(k); if (!utf8_is_valid(k)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, 0, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); return 0; } @@ -2193,7 +2193,7 @@ int config_parse_unit_requires_mounts_for( return log_oom(); if (!utf8_is_valid(n)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); continue; } @@ -3172,7 +3172,7 @@ int config_parse_namespace_path_strv( return log_oom(); if (!utf8_is_valid(n)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); continue; } diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index ed8a926a13..49513dd857 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -620,7 +620,7 @@ int config_parse_string( assert(data); if (!utf8_is_valid(rvalue)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); return 0; } @@ -658,7 +658,7 @@ int config_parse_path( assert(data); if (!utf8_is_valid(rvalue)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); return 0; } @@ -724,7 +724,7 @@ int config_parse_strv(const char *unit, return log_oom(); if (!utf8_is_valid(n)) { - log_invalid_utf8(unit, LOG_ERR, filename, line, EINVAL, rvalue); + log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue); free(n); continue; } diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 4efed138c9..fb0234baae 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -123,13 +123,6 @@ int config_parse_log_level(const char *unit, const char *filename, unsigned line int config_parse_signal(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_personality(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); -#define log_invalid_utf8(unit, level, config_file, config_line, error, rvalue) \ - do { \ - _cleanup_free_ char *_p = utf8_escape_invalid(rvalue); \ - log_syntax(unit, level, config_file, config_line, error, \ - "String is not UTF-8 clean, ignoring assignment: %s", strna(_p)); \ - } while(false) - #define DEFINE_CONFIG_PARSE_ENUM(function,name,type,msg) \ int function(const char *unit, \ const char *filename, \ -- cgit v1.2.3-54-g00ecf From 94d75d6423f16f1fb66d214cb640ae3a114c5c58 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 Sep 2015 20:28:51 +0200 Subject: conf-parser: make use of DEFINE_PARSER macro to parse mode_t Let's unify more code. Also, rework the macro to accept a trailing semicolon, to make the code prettier. --- src/shared/conf-parser.c | 72 ++++++++++++++++-------------------------------- 1 file changed, 23 insertions(+), 49 deletions(-) (limited to 'src/shared/conf-parser.c') diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 49513dd857..c282fb1231 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -415,16 +415,17 @@ int config_parse_many(const char *conf_file, } #define DEFINE_PARSER(type, vartype, conv_func) \ - int config_parse_##type(const char *unit, \ - const char *filename, \ - unsigned line, \ - const char *section, \ - unsigned section_line, \ - const char *lvalue, \ - int ltype, \ - const char *rvalue, \ - void *data, \ - void *userdata) { \ + int config_parse_##type( \ + const char *unit, \ + const char *filename, \ + unsigned line, \ + const char *section, \ + unsigned section_line, \ + const char *lvalue, \ + int ltype, \ + const char *rvalue, \ + void *data, \ + void *userdata) { \ \ vartype *i = data; \ int r; \ @@ -441,16 +442,18 @@ int config_parse_many(const char *conf_file, #type, rvalue); \ \ return 0; \ - } - -DEFINE_PARSER(int, int, safe_atoi) -DEFINE_PARSER(long, long, safe_atoli) -DEFINE_PARSER(uint32, uint32_t, safe_atou32) -DEFINE_PARSER(uint64, uint64_t, safe_atou64) -DEFINE_PARSER(unsigned, unsigned, safe_atou) -DEFINE_PARSER(double, double, safe_atod) -DEFINE_PARSER(nsec, nsec_t, parse_nsec) -DEFINE_PARSER(sec, usec_t, parse_sec) + } \ + struct __useless_struct_to_allow_trailing_semicolon__ + +DEFINE_PARSER(int, int, safe_atoi); +DEFINE_PARSER(long, long, safe_atoli); +DEFINE_PARSER(uint32, uint32_t, safe_atou32); +DEFINE_PARSER(uint64, uint64_t, safe_atou64); +DEFINE_PARSER(unsigned, unsigned, safe_atou); +DEFINE_PARSER(double, double, safe_atod); +DEFINE_PARSER(nsec, nsec_t, parse_nsec); +DEFINE_PARSER(sec, usec_t, parse_sec); +DEFINE_PARSER(mode, mode_t, parse_mode); int config_parse_iec_size(const char* unit, const char *filename, @@ -739,35 +742,6 @@ int config_parse_strv(const char *unit, return 0; } -int config_parse_mode( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - mode_t *m = data; - int r; - - assert(filename); - assert(lvalue); - assert(rvalue); - assert(data); - - r = parse_mode(rvalue, m); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse mode value, ignoring: %s", rvalue); - return 0; - } - - return 0; -} - int config_parse_log_facility( const char *unit, const char *filename, -- cgit v1.2.3-54-g00ecf