bugs in logging api and implementation
- Access g_logging_settings is not thread safe. The code in VLC is not thread safe too. https://code.videolan.org/sammirata/vlc/-/blob/librist/modules/access/rist.c
static struct rist_logging_settings *logging_settings;
p_sys->rist_log_callback_arg.p_access = (vlc_object_t *)p_access;
if (rist_logging_set(&logging_settings, i_verbose_level, log_cb, (void *)&p_sys->rist_log_callback_arg, NULL, stderr) != 0) {
msg_Err(p_access,"Could not set logging\n");
goto failed;
}
- rist_logging_set can lead to dangling pointer dereference if
logging_settings
is a pointer to uninitializedstruct rist_logging_settings *
. For example:
struct rist_logging_settings *logging_settings;
rist_logging_set(&logging_settings, ...);
-
The API is unclear whether user can allocate
struct rist_logging_settings *
by themself or not. -
rist_logging_set()
andrist_logging_settings_free()
not at the same header file.
Looks like current implementation and use case is a non-thread safe lazy-initialized singleton, but it's not clear from the API.
Edited by Zhao Zhili