ipilcher / n5550

Thecus N5550 hardware support
GNU General Public License v2.0
8 stars 8 forks source link

fcd_main_read_monitor() assumes no monitor_fn means no mutex #15

Closed ipilcher closed 4 years ago

ipilcher commented 4 years ago

fcd_main_read_monitor() assumes that only monitors that have a separate thread (as indicated by a non-zero monitor_fn) have a mutex associated with their message buffer.

static void fcd_main_read_monitor(int tty_fd, struct fcd_monitor *mon)
{
    int ret;

    if (mon->enabled) {

        if (mon->monitor_fn != 0) {
            ret = pthread_mutex_lock(&mon->mutex);
            if (ret != 0)
                FCD_PT_ABRT("pthread_mutex_lock", ret);
        }

        fcd_tty_write_msg(tty_fd, mon);

This is not true of the HDD temperature monitor; its message buffer is updated by the SMART monitor thread, but it still has a mutex, even though it does not have a separate thread (and thus has a "null" monitor_fn).

struct fcd_monitor fcd_hddtemp_monitor = {
    .mutex          = PTHREAD_MUTEX_INITIALIZER,
    .name           = "HDD temperature",
    .monitor_fn     = 0,
    .buf            = "....."
                  "HDD TEMPERATURE     "
                  "                    ",
    .enabled        = true,
    .enabled_opt_name   = "enable_hddtemp_monitor",
    .raiddisk_opts      = fcd_hddtemp_raiddisk_opts,
    .freecusd_opts      = fcd_hddtemp_freecusd_opts,
};
ipilcher commented 4 years ago

The logic in fcd_main_read_monitor() exists so that it doesn't attempt to lock/unlock a (non-existent) mutex for the "logo monitor," which displays a static message.

Given the very low cost of locking/unlocking an uncontended mutex, simplify the code by eliminating the (faulty) logic in fcd_main_read_monitor() and adding a mutex to the logo monitor.