Merge pull request #8258 from keszybz/log-issues

Fix some logging issues
This commit is contained in:
Lennart Poettering 2018-02-23 19:54:32 +01:00 committed by GitHub
commit 15eac526e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 139 additions and 117 deletions

View File

@ -83,6 +83,16 @@ static bool prohibit_ipc = false;
* use here. */
static char *log_abort_msg = NULL;
/* An assert to use in logging functions that does not call recursively
* into our logging functions (since that might lead to a loop). */
#define assert_raw(expr) \
do { \
if (_unlikely_(!(expr))) { \
fputs(#expr "\n", stderr); \
abort(); \
} \
} while (false)
static void log_close_console(void) {
if (console_fd < 0)
@ -365,7 +375,7 @@ static int write_to_console(
highlight = LOG_PRI(level) <= LOG_ERR && show_color;
if (show_location) {
xsprintf(location, "(%s:%i) ", file, line);
(void) snprintf(location, sizeof location, "(%s:%i) ", file, line);
iovec[n++] = IOVEC_MAKE_STRING(location);
}
@ -498,38 +508,40 @@ static int log_do_header(
const char *file, int line, const char *func,
const char *object_field, const char *object,
const char *extra_field, const char *extra) {
int r;
snprintf(header, size,
"PRIORITY=%i\n"
"SYSLOG_FACILITY=%i\n"
"%s%s%s"
"%s%.*i%s"
"%s%s%s"
"%s%.*i%s"
"%s%s%s"
"%s%s%s"
"SYSLOG_IDENTIFIER=%s\n",
LOG_PRI(level),
LOG_FAC(level),
isempty(file) ? "" : "CODE_FILE=",
isempty(file) ? "" : file,
isempty(file) ? "" : "\n",
line ? "CODE_LINE=" : "",
line ? 1 : 0, line, /* %.0d means no output too, special case for 0 */
line ? "\n" : "",
isempty(func) ? "" : "CODE_FUNC=",
isempty(func) ? "" : func,
isempty(func) ? "" : "\n",
error ? "ERRNO=" : "",
error ? 1 : 0, error,
error ? "\n" : "",
isempty(object) ? "" : object_field,
isempty(object) ? "" : object,
isempty(object) ? "" : "\n",
isempty(extra) ? "" : extra_field,
isempty(extra) ? "" : extra,
isempty(extra) ? "" : "\n",
program_invocation_short_name);
r = snprintf(header, size,
"PRIORITY=%i\n"
"SYSLOG_FACILITY=%i\n"
"%s%.256s%s" /* CODE_FILE */
"%s%.*i%s" /* CODE_LINE */
"%s%.256s%s" /* CODE_FUNC */
"%s%.*i%s" /* ERRNO */
"%s%.256s%s" /* object */
"%s%.256s%s" /* extra */
"SYSLOG_IDENTIFIER=%.256s\n",
LOG_PRI(level),
LOG_FAC(level),
isempty(file) ? "" : "CODE_FILE=",
isempty(file) ? "" : file,
isempty(file) ? "" : "\n",
line ? "CODE_LINE=" : "",
line ? 1 : 0, line, /* %.0d means no output too, special case for 0 */
line ? "\n" : "",
isempty(func) ? "" : "CODE_FUNC=",
isempty(func) ? "" : func,
isempty(func) ? "" : "\n",
error ? "ERRNO=" : "",
error ? 1 : 0, error,
error ? "\n" : "",
isempty(object) ? "" : object_field,
isempty(object) ? "" : object,
isempty(object) ? "" : "\n",
isempty(extra) ? "" : extra_field,
isempty(extra) ? "" : extra,
isempty(extra) ? "" : "\n",
program_invocation_short_name);
assert_raw((size_t) r < size);
return 0;
}
@ -577,11 +589,11 @@ int log_dispatch_internal(
const char *func,
const char *object_field,
const char *object,
const char *extra,
const char *extra_field,
const char *extra,
char *buffer) {
assert(buffer);
assert_raw(buffer);
if (error < 0)
error = -error;
@ -698,7 +710,7 @@ int log_internalv_realm(
if (error != 0)
errno = error;
vsnprintf(buffer, sizeof(buffer), format, ap);
(void) vsnprintf(buffer, sizeof buffer, format, ap);
return log_dispatch_internal(level, error, file, line, func, NULL, NULL, NULL, NULL, buffer);
}
@ -721,7 +733,8 @@ int log_internal_realm(
return r;
}
int log_object_internalv(
_printf_(10,0)
static int log_object_internalv(
int level,
int error,
const char *file,
@ -757,7 +770,7 @@ int log_object_internalv(
} else
b = buffer = newa(char, LINE_MAX);
vsnprintf(b, LINE_MAX, format, ap);
(void) vsnprintf(b, LINE_MAX, format, ap);
return log_dispatch_internal(level, error, file, line, func,
object_field, object, extra_field, extra, buffer);
@ -800,7 +813,7 @@ static void log_assert(
return;
DISABLE_WARNING_FORMAT_NONLITERAL;
xsprintf(buffer, format, text, file, line, func);
(void) snprintf(buffer, sizeof buffer, format, text, file, line, func);
REENABLE_WARNING;
log_abort_msg = buffer;
@ -974,7 +987,7 @@ int log_struct_internal(
errno = error;
va_copy(aq, ap);
vsnprintf(buf, sizeof(buf), format, aq);
(void) vsnprintf(buf, sizeof buf, format, aq);
va_end(aq);
if (startswith(buf, "MESSAGE=")) {
@ -1273,7 +1286,7 @@ int log_syntax_internal(
errno = error;
va_start(ap, format);
vsnprintf(buffer, sizeof(buffer), format, ap);
(void) vsnprintf(buffer, sizeof buffer, format, ap);
va_end(ap);
if (unit)

View File

@ -144,19 +144,6 @@ int log_object_internal(
const char *extra,
const char *format, ...) _printf_(10,11);
int log_object_internalv(
int level,
int error,
const char *file,
int line,
const char *func,
const char *object_field,
const char *object,
const char *extra_field,
const char *extra,
const char *format,
va_list ap) _printf_(10,0);
int log_struct_internal(
int level,
int error,

View File

@ -27,9 +27,11 @@
#include "macro.h"
#define xsprintf(buf, fmt, ...) \
assert_message_se((size_t) snprintf(buf, ELEMENTSOF(buf), fmt, __VA_ARGS__) < ELEMENTSOF(buf), "xsprintf: " #buf "[] must be big enough")
#define snprintf_ok(buf, len, fmt, ...) \
((size_t) snprintf(buf, len, fmt, __VA_ARGS__) < (len))
#define xsprintf(buf, fmt, ...) \
assert_message_se(snprintf_ok(buf, ELEMENTSOF(buf), fmt, __VA_ARGS__), "xsprintf: " #buf "[] must be big enough")
#define VA_FORMAT_ADVANCE(format, ap) \
do { \

View File

@ -1699,7 +1699,7 @@ static void unit_status_log_starting_stopping_reloading(Unit *u, JobType t) {
format = unit_get_status_message_format(u, t);
DISABLE_WARNING_FORMAT_NONLITERAL;
xsprintf(buf, format, unit_description(u));
snprintf(buf, sizeof buf, format, unit_description(u));
REENABLE_WARNING;
mid = t == JOB_START ? "MESSAGE_ID=" SD_MESSAGE_UNIT_STARTING_STR :

View File

@ -22,8 +22,6 @@
#include "journald-server.h"
bool valid_user_field(const char *p, size_t l, bool allow_protected);
void server_process_native_message(Server *s, const void *buffer, size_t buffer_size, const struct ucred *ucred, const struct timeval *tv, const char *label, size_t label_len);
void server_process_native_file(Server *s, int fd, const struct ucred *ucred, const struct timeval *tv, const char *label, size_t label_len);

View File

@ -2942,7 +2942,8 @@ static int start_unit_one(
log_error("Failed to %s %s: %s", verb, name, bus_error_message(error, r));
if (!sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) &&
!sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED))
!sd_bus_error_has_name(error, BUS_ERROR_UNIT_MASKED) &&
!sd_bus_error_has_name(error, BUS_ERROR_JOB_TYPE_NOT_APPLICABLE))
log_error("See %s logs and 'systemctl%s status%s %s' for details.",
arg_scope == UNIT_FILE_SYSTEM ? "system" : "user",
arg_scope == UNIT_FILE_SYSTEM ? "" : " --user",

View File

@ -35,19 +35,18 @@ assert_cc((LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, LOG_LOCAL3 | LOG_DEBUG) & LOG
assert_cc((LOG_REALM_PLUS_LEVEL(LOG_REALM_UDEV, LOG_USER | LOG_INFO) & LOG_PRIMASK)
== LOG_INFO);
int main(int argc, char* argv[]) {
log_set_target(LOG_TARGET_CONSOLE);
log_open();
#define X10(x) x x x x x x x x x x
#define X100(x) X10(X10(x))
#define X1000(x) X100(X10(x))
static void test_log_console(void) {
log_struct(LOG_INFO,
"MESSAGE=Waldo PID="PID_FMT, getpid_cached(),
"SERVICE=piepapo",
NULL);
}
log_set_target(LOG_TARGET_JOURNAL);
log_open();
static void test_log_journal(void) {
log_struct(LOG_INFO,
"MESSAGE=Foobar PID="PID_FMT, getpid_cached(),
"SERVICE=foobar",
@ -59,6 +58,32 @@ int main(int argc, char* argv[]) {
(int) 1, 'A', (short) 2, (long int) 3, (long long int) 4, (void*) 1, "foo", (float) 2.5f, (double) 3.5, (long double) 4.5,
"SUFFIX=GOT IT",
NULL);
}
static void test_long_lines(void) {
log_object_internal(LOG_NOTICE,
EUCLEAN,
X1000("abcd_") ".txt",
1000000,
X1000("fff") "unc",
"OBJECT=",
X1000("obj_") "ect",
"EXTRA=",
X1000("ext_") "tra",
"asdfasdf %s asdfasdfa", "foobar");
}
int main(int argc, char* argv[]) {
int target;
for (target = 0; target < _LOG_TARGET_MAX; target++) {
log_set_target(target);
log_open();
test_log_console();
test_log_journal();
test_long_lines();
}
return 0;
}

View File

@ -94,7 +94,7 @@ static int prepare(char *dir, char *filename)
if (r < 0 && errno != EEXIST)
return -errno;
xsprintf(buf, "%s/%s", dir, filename);
snprintf(buf, sizeof buf, "%s/%s", dir, filename);
fd = open(buf, O_RDWR|O_CREAT|O_CLOEXEC, S_IRUSR|S_IWUSR);
if (fd < 0)

View File

@ -115,6 +115,7 @@
#include "stdio-util.h"
#include "string-util.h"
#include "udev.h"
#include "udev-util.h"
#define ONBOARD_INDEX_MAX (16*1024-1)
@ -236,11 +237,11 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
size_t l;
char *s;
const char *attr, *port_name;
struct udev_device *pci = NULL;
_cleanup_udev_device_unref_ struct udev_device *pci = NULL;
char slots[PATH_MAX];
_cleanup_closedir_ DIR *dir = NULL;
struct dirent *dent;
int hotplug_slot = 0, err = 0;
int hotplug_slot = 0;
if (sscanf(udev_device_get_sysname(names->pcidev), "%x:%x:%x.%u", &domain, &bus, &slot, &func) != 4)
return -ENOENT;
@ -270,21 +271,20 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
/* ACPI _SUN — slot user number */
pci = udev_device_new_from_subsystem_sysname(udev, "subsystem", "pci");
if (!pci) {
err = -ENOENT;
goto out;
}
if (!pci)
return -ENOENT;
if (!snprintf_ok(slots, sizeof slots, "%s/slots", udev_device_get_syspath(pci)))
return -ENAMETOOLONG;
xsprintf(slots, "%s/slots", udev_device_get_syspath(pci));
dir = opendir(slots);
if (!dir) {
err = -errno;
goto out;
}
if (!dir)
return -errno;
FOREACH_DIRENT_ALL(dent, dir, break) {
int i;
char *rest, *address, str[PATH_MAX];
char *rest, str[PATH_MAX];
_cleanup_free_ char *address = NULL;
if (dent->d_name[0] == '.')
continue;
@ -294,13 +294,11 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
if (i < 1)
continue;
xsprintf(str, "%s/%s/address", slots, dent->d_name);
if (read_one_line_file(str, &address) >= 0) {
if (snprintf_ok(str, sizeof str, "%s/%s/address", slots, dent->d_name) &&
read_one_line_file(str, &address) >= 0)
/* match slot address with device by stripping the function */
if (strneq(address, udev_device_get_sysname(names->pcidev), strlen(address)))
if (streq(address, udev_device_get_sysname(names->pcidev)))
hotplug_slot = i;
free(address);
}
if (hotplug_slot > 0)
break;
@ -321,9 +319,8 @@ static int dev_pci_slot(struct udev_device *dev, struct netnames *names) {
if (l == 0)
names->pci_slot[0] = '\0';
}
out:
udev_device_unref(pci);
return err;
return 0;
}
static int names_vio(struct udev_device *dev, struct netnames *names) {
@ -517,7 +514,6 @@ static int names_ccw(struct udev_device *dev, struct netnames *names) {
const char *bus_id, *subsys;
size_t bus_id_len;
size_t bus_id_start;
int rc;
assert(dev);
assert(names);
@ -559,9 +555,9 @@ static int names_ccw(struct udev_device *dev, struct netnames *names) {
bus_id += bus_id_start < bus_id_len ? bus_id_start : bus_id_len - 1;
/* Store the CCW bus-ID for use as network device name */
rc = snprintf(names->ccw_busid, sizeof(names->ccw_busid), "c%s", bus_id);
if (rc >= 0 && rc < (int)sizeof(names->ccw_busid))
if (snprintf_ok(names->ccw_busid, sizeof(names->ccw_busid), "c%s", bus_id))
names->type = NET_CCW;
return 0;
}
@ -674,7 +670,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (err >= 0 && names.type == NET_CCW) {
char str[IFNAMSIZ];
if (snprintf(str, sizeof(str), "%s%s", prefix, names.ccw_busid) < (int)sizeof(str))
if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.ccw_busid))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
goto out;
}
@ -684,7 +680,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (err >= 0 && names.type == NET_VIO) {
char str[IFNAMSIZ];
if (snprintf(str, sizeof(str), "%s%s", prefix, names.vio_slot) < (int)sizeof(str))
if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.vio_slot))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
goto out;
}
@ -694,7 +690,7 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (err >= 0 && names.type == NET_PLATFORM) {
char str[IFNAMSIZ];
if (snprintf(str, sizeof(str), "%s%s", prefix, names.platform_path) < (int)sizeof(str))
if (snprintf_ok(str, sizeof str, "%s%s", prefix, names.platform_path))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
goto out;
}
@ -708,21 +704,21 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (names.type == NET_PCI) {
char str[IFNAMSIZ];
if (names.pci_onboard[0])
if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_onboard) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_ONBOARD", str);
if (names.pci_onboard[0] &&
snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_onboard))
udev_builtin_add_property(dev, test, "ID_NET_NAME_ONBOARD", str);
if (names.pci_onboard_label)
if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_onboard_label) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_LABEL_ONBOARD", str);
if (names.pci_onboard_label &&
snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_onboard_label))
udev_builtin_add_property(dev, test, "ID_NET_LABEL_ONBOARD", str);
if (names.pci_path[0])
if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_path) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_path[0] &&
snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_path))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_slot[0])
if (snprintf(str, sizeof(str), "%s%s", prefix, names.pci_slot) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
if (names.pci_slot[0] &&
snprintf_ok(str, sizeof str, "%s%s", prefix, names.pci_slot))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
goto out;
}
@ -731,13 +727,13 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (err >= 0 && names.type == NET_USB) {
char str[IFNAMSIZ];
if (names.pci_path[0])
if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_path, names.usb_ports) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_path[0] &&
snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_path, names.usb_ports))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_slot[0])
if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_slot, names.usb_ports) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
if (names.pci_slot[0] &&
snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_slot, names.usb_ports))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
goto out;
}
@ -746,13 +742,13 @@ static int builtin_net_id(struct udev_device *dev, int argc, char *argv[], bool
if (err >= 0 && names.type == NET_BCMA) {
char str[IFNAMSIZ];
if (names.pci_path[0])
if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_path, names.bcma_core) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_path[0] &&
snprintf_ok(str, sizeof str, "%s%s%s", prefix, names.pci_path, names.bcma_core))
udev_builtin_add_property(dev, test, "ID_NET_NAME_PATH", str);
if (names.pci_slot[0])
if (snprintf(str, sizeof(str), "%s%s%s", prefix, names.pci_slot, names.bcma_core) < (int)sizeof(str))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
if (names.pci_slot[0] &&
snprintf(str, sizeof str, "%s%s%s", prefix, names.pci_slot, names.bcma_core))
udev_builtin_add_property(dev, test, "ID_NET_NAME_SLOT", str);
goto out;
}
out: