Improve logging and termination

cmake/ConkyPlatformChecks.cmake

@@ -679,13 +679,9 @@ if(BUILD_COLOUR_NAME_MAP)
   mark_as_advanced(APP_GPERF)
 endif(BUILD_COLOUR_NAME_MAP)
 
-if(CMAKE_BUILD_TYPE MATCHES "Debug")
-  set(DEBUG true)
-endif(CMAKE_BUILD_TYPE MATCHES "Debug")
-
 # The version numbers are simply derived from the date and number of commits
 # since start of month
-if(DEBUG)
+if(CMAKE_BUILD_TYPE MATCHES "Debug")
   execute_process(COMMAND ${APP_GIT} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git
     log --since=${VERSION_MAJOR}-${VERSION_MINOR}-01
     --pretty=oneline
@@ -694,4 +690,4 @@ if(DEBUG)
     RESULT_VARIABLE RETVAL
     OUTPUT_VARIABLE COMMIT_COUNT
     OUTPUT_STRIP_TRAILING_WHITESPACE)
-endif(DEBUG)
+endif()

cmake/config.h.in

@@ -6,8 +6,6 @@
 #ifndef _conky_config_h_
 #define _conky_config_h_
 
-#cmakedefine DEBUG
-
 #define SYSTEM_NAME "@CMAKE_SYSTEM_NAME@"
 #define PACKAGE_NAME "@PROJECT_NAME@"
 #define VERSION "@VERSION@"

lua/libcairo_imlib2_helper.h

@@ -28,27 +28,42 @@
 #include <Imlib2.h>
 #include <cairo.h>
 
-#include "logging.h"
-
-void cairo_place_image(const char *file, cairo_t *cr, int x, int y,
-                       int width, int height, double alpha) {
+#include "config.h"
+
+#ifdef BUILD_I18N
+#include <libintl.h>
+#else
+#define gettext
+#endif
+
+// TODO: inject reference to conky logger
+// Lua allows modifying .so loading, so for each loaded library check if it has
+// some hardcoded set_logger function symbol, and call it to set per-library
+// reference to the global logger.
+#define PRINT_ERROR(Format, ...)                   \
+  fputs(stderr, "cairoimagehelper: ");             \
+  fprintf(stderr, gettext(Format), ##__VA_ARGS__); \
+  fputs(stderr, "\n")
+
+void cairo_place_image(const char *file, cairo_t *cr, int x, int y, int width,
+                       int height, double alpha) {
   int w, h, stride;
   Imlib_Image alpha_image, image, premul;
   cairo_surface_t *result;
 
   if (!file) {
-    NORM_ERR("cairoimagehelper: File is NULL\n");
+    PRINT_ERROR("File is nil");
     return;
   }
 
   if (!cr) {
-    NORM_ERR("cairoimagehelper: cairo_t is NULL\n");
+    PRINT_ERROR("cairo_t is nil");
     return;
   }
 
   image = (Imlib_Image *)imlib_load_image(file);
   if (!image) {
-    NORM_ERR("cairoimagehelper: Couldn't load %s\n", file);
+    PRINT_ERROR("can't load %s", file);
     return;
   }
 
@@ -57,7 +72,7 @@ void cairo_place_image(const char *file, cairo_t *cr, int x, int y,
   h = imlib_image_get_height();
 
   if ((w <= 0) && (h <= 0)) {
-    NORM_ERR("cairoimagehelper: %s has 0 size\n", file);
+    PRINT_ERROR("%s has 0 size", file);
     return;
   }
 
@@ -67,7 +82,7 @@ void cairo_place_image(const char *file, cairo_t *cr, int x, int y,
   /* create temporary image */
   premul = imlib_create_image(width, height);
   if (!premul) {
-    NORM_ERR("cairoimagehelper: Couldn't create premul image for %s\n", file);
+    PRINT_ERROR("can't create premul image for %s", file);
     return;
   }
 
@@ -83,12 +98,12 @@ void cairo_place_image(const char *file, cairo_t *cr, int x, int y,
   /* and use the alpha channel of the source image */
   imlib_image_copy_alpha_to_image(alpha_image, 0, 0);
 
-  stride = cairo_format_stride_for_width (CAIRO_FORMAT_ARGB32, width);
+  stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, width);
 
   /* now pass the result to cairo */
   result = cairo_image_surface_create_for_data(
-      (unsigned char  *)imlib_image_get_data_for_reading_only(), CAIRO_FORMAT_ARGB32,
-      width, height, stride);
+      (unsigned char *)imlib_image_get_data_for_reading_only(),
+      CAIRO_FORMAT_ARGB32, width, height, stride);
 
   cairo_set_source_surface(cr, result, x, y);
   cairo_paint_with_alpha(cr, alpha);
@@ -101,7 +116,6 @@ void cairo_place_image(const char *file, cairo_t *cr, int x, int y,
   imlib_free_image();
 
   cairo_surface_destroy(result);
-
 }
 
 void cairo_draw_image(const char *file, cairo_surface_t *cs, int x, int y,
@@ -112,23 +126,23 @@ void cairo_draw_image(const char *file, cairo_surface_t *cs, int x, int y,
   double scaled_w, scaled_h;
 
   if (!file) {
-    NORM_ERR("cairoimagehelper: File is NULL\n");
+    PRINT_ERROR("File is nil");
     return;
   }
 
   if (!cs) {
-    NORM_ERR("cairoimagehelper: Surface is NULL\n");
+    PRINT_ERROR("Surface is nil");
     return;
   }
 
   if ((scale_x <= 0.0) && (scale_y <= 0.0)) {
-    NORM_ERR("cairoimagehelper: Image Scale is 0, %s\n", file);
+    PRINT_ERROR("Image Scale is 0, %s", file);
     return;
   }
 
   Imlib_Image *image = (Imlib_Image *)imlib_load_image(file);
   if (!image) {
-    NORM_ERR("cairoimagehelper: Couldn't load %s\n", file);
+    PRINT_ERROR("Couldn't load %s", file);
     return;
   }
 
@@ -137,15 +151,15 @@ void cairo_draw_image(const char *file, cairo_surface_t *cs, int x, int y,
   h = imlib_image_get_height();
 
   if ((w <= 0) && (h <= 0)) {
-    NORM_ERR("cairoimagehelper: %s has 0 size\n", file);
+    PRINT_ERROR("%s has 0 size", file);
     return;
   }
 
   scaled_w = *return_scale_w = scale_x * (double)w;
   scaled_h = *return_scale_h = scale_y * (double)h;
 
   if ((scaled_w <= 0.0) && (scaled_h <= 0.0)) {
-    NORM_ERR("cairoimagehelper: %s scaled image has 0 size\n", file);
+    PRINT_ERROR("%s scaled image has 0 size", file);
     return;
   }
 

src/CMakeLists.txt

@@ -62,8 +62,21 @@ execute_process(
   COMMAND ${APP_GPERF} --ignore-case -LC++ -Zcolor_name_hash -t -7 -m1 -C -E
 )
 
+# Separate logger library to allow optional artifacts like lua libraries to use
+# it at some point.
+# With this intent, logger shouldn't depend on anything to be built before it.
+add_library(conky_logger
+  str_buffer.cc
+  str_buffer.hh
+  logger.cc
+  logger.hh
+)
+target_include_directories(conky_logger PRIVATE ${CMAKE_BINARY_DIR}) # config.h only
+set(conky_libs ${conky_libs} conky_logger)
+
 set(conky_sources
   ${conky_sources}
+  logging.cc
   c++wrap.cc
   c++wrap.hh
   colour-settings.cc

src/algebra.cc

@@ -157,15 +157,15 @@ char *arg_to_string(const char *arg) {
 double arg_to_double(const char *arg) {
   double d;
   if (sscanf(arg, "%lf", &d) != 1) {
-    NORM_ERR("converting '%s' to double failed", arg);
+    LOG_WARNING("invalid double string: '%s'", arg);
     return 0.0;
   }
   return d;
 }
 long arg_to_long(const char *arg) {
   long l;
   if (sscanf(arg, "%ld", &l) != 1) {
-    NORM_ERR("converting '%s' to long failed", arg);
+    LOG_WARNING("invalid long (int) string: '%s'", arg);
     return 0;
   }
   return l;
@@ -181,7 +181,7 @@ int compare(const char *expr) {
   mtype = get_match_type(expr);
 
   if ((idx <= 0) || mtype == -1) {
-    NORM_ERR("failed to parse compare string '%s'", expr);
+    LOG_WARNING("failed to parse compare string: '%s'", expr);
     return -2;
   }
 
@@ -192,15 +192,15 @@ int compare(const char *expr) {
   type1 = get_arg_type(expr_dup);
   type2 = get_arg_type(expr_dup + idx + 1);
   if (type1 == ARG_BAD || type2 == ARG_BAD) {
-    NORM_ERR("Bad arguments: '%s' and '%s'", expr_dup, (expr_dup + idx + 1));
+    LOG_WARNING("bad arguments: '%s' and '%s'", expr_dup, (expr_dup + idx + 1));
     free(expr_dup);
     return -2;
   }
   if (type1 == ARG_LONG && type2 == ARG_DOUBLE) { type1 = ARG_DOUBLE; }
   if (type1 == ARG_DOUBLE && type2 == ARG_LONG) { type2 = ARG_DOUBLE; }
   if (type1 != type2) {
-    NORM_ERR("trying to compare args '%s' and '%s' of different type", expr_dup,
-             (expr_dup + idx + 1));
+    LOG_WARNING("trying to compare args '%s' and '%s' of different type",
+                expr_dup, (expr_dup + idx + 1));
     free(expr_dup);
     return -2;
   }
@@ -239,11 +239,11 @@ int check_if_match(struct text_object *obj) {
 
   generate_text_internal(expression.get(), max_user_text.get(*state),
                          *obj->sub);
-  DBGP("parsed arg into '%s'", expression.get());
+  LOG_TRACE("parsed arg into '%s'", expression.get());
 
   val = compare(expression.get());
   if (val == -2) {
-    NORM_ERR("compare failed for expression '%s'", expression.get());
+    LOG_WARNING("compare failed for expression '%s'", expression.get());
   } else if (val == 0) {
     result = 0;
   }

src/apcupsd.cc

@@ -202,7 +202,7 @@ int update_apcupsd() {
     snprintf(portbuf, 8, "%d", apcupsd.port);
     res = getaddrinfo(apcupsd.host, portbuf, &hints, &ai);
     if (res != 0) {
-      NORM_ERR("APCUPSD getaddrinfo: %s", gai_strerror(res));
+      LOG_ERROR("unable to get APCUPSD address info: %s", gai_strerror(res));
       break;
     }
     for (rp = ai; rp != nullptr; rp = rp->ai_next) {

src/ccurl_thread.cc

@@ -29,7 +29,7 @@
 #include "logging.h"
 #include "text_object.h"
 
-#ifdef DEBUG
+#ifndef NDEBUG
 #include <assert.h>
 #endif /* DEBUG */
 
@@ -76,7 +76,8 @@ size_t curl_internal::write_cb(void *ptr, size_t size, size_t nmemb,
   return realsize;
 }
 
-curl_internal::curl_internal(const std::string &url) : curl(curl_easy_init()) {
+curl_internal::curl_internal(const std::string &url)
+    : curl(curl_easy_init()), url(std::string(url)) {
   if (!curl) throw std::runtime_error("curl_easy_init() failed");
 
   curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1);
@@ -132,15 +133,18 @@ void curl_internal::do_work() {
         case 304:
           break;
         default:
-          NORM_ERR("curl: no data from server, got HTTP status %ld",
-                   http_status_code);
+          LOG_WARNING(
+              "no response data from server for '%s'; response status: %ld",
+              this->url.c_str(), http_status_code);
           break;
       }
     } else {
-      NORM_ERR("curl: no HTTP status from server");
+      LOG_WARNING("no HTTP response from server for '%s'",
+                          this->url.c_str());
     }
   } else {
-    NORM_ERR("curl: could not retrieve data from server");
+    LOG_WARNING("can't retrieve data from server for '%s'",
+                        this->url.c_str());
   }
 }
 }  // namespace priv
@@ -185,7 +189,7 @@ void curl_parse_arg(struct text_object *obj, const char *arg) {
   char *space;
 
   if (strlen(arg) < 1) {
-    NORM_ERR("wrong number of arguments for $curl");
+    LOG_WARNING("wrong number of arguments for $curl");
     return;
   }
 
@@ -212,7 +216,7 @@ void curl_print(struct text_object *obj, char *p, unsigned int p_max_size) {
   struct curl_data *cd = static_cast<struct curl_data *>(obj->data.opaque);
 
   if (!cd) {
-    NORM_ERR("error processing Curl data");
+    LOG_ERROR("error processing curl data");
     return;
   }
   ccurl_process_info(p, p_max_size, cd->uri, cd->interval);

src/ccurl_thread.h

@@ -33,6 +33,7 @@
 namespace priv {
 // factored out stuff that does not depend on the template parameters
 class curl_internal {
+  std::string url;
  public:
   std::string last_modified;
   std::string etag;

src/colours.cc

@@ -83,17 +83,13 @@ std::optional<Colour> parse_hex_color(const std::string &color) {
     }
     return -1;
   };
-  const auto none = [&]() {
-    NORM_ERR("can't parse hex color '%s' (%d)", name, len);
-    return std::nullopt;
-  };
 
   uint8_t argb[4] = {0xff, 0, 0, 0};
   if (len == 3 || len == 4) {
     bool skip_alpha = (len == 3);
     for (size_t i = 0; i < len; i++) {
       int nib = hex_nibble_value(name[i]);
-      if (nib < 0) { return none(); }
+      if (nib < 0) { return std::nullopt; }
       // Duplicate the nibble, so "#abc" -> 0xaa, 0xbb, 0xcc
       int val = (nib << 4) + nib;
 
@@ -104,13 +100,13 @@ std::optional<Colour> parse_hex_color(const std::string &color) {
     for (size_t i = 0; i + 1 < len; i += 2) {
       int nib1 = hex_nibble_value(name[i]);
       int nib2 = hex_nibble_value(name[i + 1]);
-      if (nib1 < 0 || nib2 < 0) { return none(); }
+      if (nib1 < 0 || nib2 < 0) { return std::nullopt; }
       int val = (nib1 << 4) + nib2;
 
       argb[skip_alpha + i / 2] = val;
     }
   } else {
-    return none();
+    return std::nullopt;
   }
 
   return Colour(argb[1], argb[2], argb[3], argb[0]);
@@ -128,6 +124,8 @@ Colour parse_color(const std::string &color) {
 
 #undef TRY_PARSER
 
+  LOG_WARNING("can't parse color '%s'", color.c_str());
+
   return ERROR_COLOUR;
 }