Skip to content

bugs in logging api and implementation

  1. 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;
    }
  1. rist_logging_set can lead to dangling pointer dereference if logging_settings is a pointer to uninitialized struct rist_logging_settings *. For example:
struct rist_logging_settings *logging_settings;
rist_logging_set(&logging_settings, ...);
  1. The API is unclear whether user can allocate struct rist_logging_settings * by themself or not.

  2. rist_logging_set() and rist_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