Skip to content

Commit

Permalink
improve init error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
vmatare committed Nov 19, 2022
1 parent 92e65dd commit 8436612
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 38 deletions.
16 changes: 9 additions & 7 deletions src/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,17 @@ Driver::Driver(bool optional, unsigned int max_errors)
void Driver::try_init()
{
robust_op(
[&] () {
[&]/* op_fn */() {
if (!available())
path_.emplace(lookup());
init();
initialized_ = true;
},
[&] (const ExpectedError &e) { /* skip_fn */
if (!optional())
log(TF_WRN)
<< "Error " << errors() << "/" << max_errors()
<< " while initializing driver: " << e.what() << flush
;
[&]/* skip_fn */(const ExpectedError &e) {
log(optional() ? TF_DBG : TF_INF) << "Ignoring error ";
if (max_errors() && !optional())
log() << errors() << "/" << max_errors() << " ";
log() << "while initializing " << type_name() << ": " << e.what() << flush;
}
);
}
Expand All @@ -58,6 +57,9 @@ void Driver::robust_op(FN<void ()> op_fn, FN<void (const ExpectedError &)> skip_
errors_++;
op_fn();
errors_ = 0;
} catch (DriverInitError &e) {
e.set_context(type_name());
handle_io_error_(e, skip_fn);
} catch (SystemError &e) {
handle_io_error_(e, skip_fn);
} catch (IOerror &e) {
Expand Down
3 changes: 3 additions & 0 deletions src/driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class Driver {
* The string returned by this will be accessible via the @a path() method. */
virtual string lookup() = 0;

/// @return A user-friendly name for the type of driver represented by the implementor
virtual string type_name() const = 0;

virtual void skip_io_error(const ExpectedError &);

opt<const string> path_;
Expand Down
3 changes: 3 additions & 0 deletions src/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,8 @@ MissingEntry::MissingEntry(const string &entry)
InvocationError::InvocationError(const string &message)
: ExpectedError("Invalid command line: " + message) {}

void DriverInitError::set_context(const string &context)
{ msg_ = context + ": " + msg_; }


}
6 changes: 1 addition & 5 deletions src/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,8 @@ class SystemError : public ExpectedError {
class DriverInitError : public SystemError {
public:
using SystemError::SystemError;
};


class DriverIOError : public SystemError {
public:
using SystemError::SystemError;
void set_context(const string &context);
};


Expand Down
6 changes: 6 additions & 0 deletions src/fans.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ string TpFanDriver::lookup()
return path_;
}

string TpFanDriver::type_name() const
{ return "tpacpi fan driver"; }


/*----------------------------------------------------------------------------
| HwmonFanDriver: Driver for PWM fans, typically somewhere in sysfs. |
Expand Down Expand Up @@ -255,6 +258,9 @@ void HwmonFanDriver::init()
string HwmonFanDriver::lookup()
{ return hwmon_interface_->lookup(); }

string HwmonFanDriver::type_name() const
{ return "hwmon fan driver"; }


void HwmonFanDriver::set_speed(const Level &level)
{
Expand Down
2 changes: 2 additions & 0 deletions src/fans.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class TpFanDriver : public FanDriver {
protected:
virtual void init() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
const string path_;
Expand All @@ -93,6 +94,7 @@ class HwmonFanDriver : public FanDriver {
protected:
virtual void init() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
shared_ptr<HwmonInterface<FanDriver>> hwmon_interface_;
Expand Down
42 changes: 22 additions & 20 deletions src/message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@ LogLevel &operator++(LogLevel &l)
}


Logger &log(LogLevel lvl)
{
Logger::instance().level(lvl);
return Logger::instance();
}


Logger &flush(Logger &l)
{ return l.flush(); }

Logger &log()
{ return Logger::instance(); }





Logger::Logger()
: syslog_(false),
log_lvl_(DEFAULT_LOG_LVL),
Expand All @@ -78,16 +95,6 @@ LogLevel &Logger::log_lvl()
{ return log_lvl_; }


Logger &flush(Logger &l) { return l.flush(); }


Logger &log(LogLevel lvl)
{
Logger::instance().level(lvl);
return Logger::instance();
}


Logger::~Logger()
{
flush();
Expand All @@ -106,19 +113,13 @@ void Logger::enable_syslog()

Logger &Logger::flush()
{
if (msg_pfx_.length() == 0) return *this;
if (msg_pfx_.length() == 0)
return *this;
if (msg_lvl_ <= log_lvl_) {
if (syslog_) {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-security"
// I think we can safely do this because thinkfan doesn't receive
// any data from unprivileged processes.
if (syslog_)
syslog(msg_lvl_, "%s", msg_pfx_.c_str());
#pragma GCC diagnostic pop
}
else {
else
std::cerr << msg_pfx_ << std::endl;
}
}
msg_pfx_ = "";

Expand Down Expand Up @@ -196,4 +197,5 @@ Logger &Logger::operator<< (const vector<unique_ptr<FanConfig>> &fan_configs)




}
6 changes: 5 additions & 1 deletion src/message.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ class Logger {
std::exception_ptr exception_;
};


Logger &flush(Logger &l);
Logger &log();
Logger &log(LogLevel lvl);

template<class ErrT, class... ArgTs> void error(const ArgTs &... args) {
Expand All @@ -104,7 +106,9 @@ template<class ErrT, class... ArgTs> void error(const ArgTs &... args) {
log(TF_ERR) << ErrT(args...).what() << flush;
}

}

} // namespace thinkfan


#ifdef USE_ATASMART
#define DND_DISK_HELP \
Expand Down
17 changes: 14 additions & 3 deletions src/sensors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ void HwmonSensorDriver::read_temps_()
string HwmonSensorDriver::lookup()
{ return hwmon_interface_->lookup(); }

string HwmonSensorDriver::type_name() const
{ return "hwmon sensor driver"; }


/*----------------------------------------------------------------------------
| TpSensorDriver: A driver for sensors provided by thinkpad_acpi, typically |
Expand Down Expand Up @@ -272,6 +275,9 @@ string TpSensorDriver::lookup()
throw IOerror(MSG_SENSOR_INIT(path()), errno);
}

string TpSensorDriver::type_name() const
{ return "tpacpi sensor driver"; }


#ifdef USE_ATASMART
/*----------------------------------------------------------------------------
Expand Down Expand Up @@ -343,6 +349,9 @@ void AtasmartSensorDriver::read_temps_()
string AtasmartSensorDriver::lookup()
{ return device_path_; }

string AtasmartSensorDriver::type_name() const
{ return "atasmart sensor driver"; }

#endif /* USE_ATASMART */


Expand Down Expand Up @@ -421,6 +430,9 @@ void NvmlSensorDriver::read_temps_()
string NvmlSensorDriver::lookup()
{ return bus_id_; }

string NvmlSensorDriver::type_name() const
{ return "NVML sensor driver"; }

#endif /* USE_NVML */


Expand Down Expand Up @@ -469,7 +481,6 @@ void LMSensorsDriver::set_unavailable()
{ path_.reset(); }



string LMSensorsDriver::lookup()
{
if (!libsensors_iface_)
Expand All @@ -481,8 +492,8 @@ string LMSensorsDriver::lookup()
}




string LMSensorsDriver::type_name() const
{ return "libsensors sensor driver"; }


void LMSensorsDriver::read_temps_()
Expand Down
9 changes: 9 additions & 0 deletions src/sensors.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class HwmonSensorDriver : public SensorDriver {
protected:
virtual void read_temps_() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
shared_ptr<HwmonInterface<SensorDriver>> hwmon_interface_;
Expand All @@ -113,6 +114,7 @@ class TpSensorDriver : public SensorDriver {
virtual void init() override;
virtual void read_temps_() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
std::char_traits<char>::off_type skip_bytes_;
Expand All @@ -128,10 +130,13 @@ class AtasmartSensorDriver : public SensorDriver {
public:
AtasmartSensorDriver(string device_path, bool optional, opt<vector<int>> correction = nullopt, opt<unsigned int> max_errors = nullopt);
virtual ~AtasmartSensorDriver();

protected:
virtual void init() override;
virtual void read_temps_() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
SkDisk *disk_;
const string device_path_;
Expand All @@ -144,10 +149,13 @@ class NvmlSensorDriver : public SensorDriver {
public:
NvmlSensorDriver(string bus_id, bool optional, opt<vector<int>> correction = nullopt, opt<unsigned int> max_errors = nullopt);
virtual ~NvmlSensorDriver() noexcept(false) override;

protected:
virtual void init() override;
virtual void read_temps_() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
const string bus_id_;
nvmlDevice_t device_;
Expand Down Expand Up @@ -185,6 +193,7 @@ class LMSensorsDriver : public SensorDriver {
virtual void init() override;
virtual void read_temps_() override;
virtual string lookup() override;
virtual string type_name() const override;

private:
const string chip_name_;
Expand Down
5 changes: 3 additions & 2 deletions src/thinkfan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,11 @@ int main(int argc, char **argv) {
#endif

if (daemonize) {
LogLevel old_lvl = Logger::instance().log_lvl();
{
// Test the config before forking
unique_ptr<const Config> test_cfg(Config::read_config(config_files));

LogLevel old_lvl = Logger::instance().log_lvl();
Logger::instance().log_lvl() = TF_ERR;

temp_state = TemperatureState(test_cfg->num_temps());
Expand All @@ -373,9 +373,10 @@ int main(int argc, char **argv) {
for (auto &sensor : test_cfg->sensors())
sensor->read_temps();

Logger::instance().log_lvl() = old_lvl;
// Own scope so the config gets destroyed before forking
}
Logger::instance().log_lvl() = old_lvl;


pid_t child_pid = ::fork();
if (child_pid < 0) {
Expand Down

0 comments on commit 8436612

Please sign in to comment.