Merge branch 'refactor-actions'

.gitignore

@@ -9,6 +9,7 @@
 /src/failsafe.o
 /src/init.o
 /src/config.o
+/src/action.o
 /src/dlhook.o
 
 /src/menu.o

Makefile

@@ -71,7 +71,7 @@ override GENERATED += KoboRoot.tgz
 src/libnm.so: override CFLAGS   += $(PTHREAD_CFLAGS) -fPIC
 src/libnm.so: override CXXFLAGS += $(PTHREAD_CFLAGS) $(QT5CORE_CFLAGS) $(QT5WIDGETS_CFLAGS) -fPIC
 src/libnm.so: override LDFLAGS  += $(PTHREAD_LIBS) $(QT5CORE_LIBS) $(QT5WIDGETS_LIBS) -ldl -Wl,-soname,libnm.so
-src/libnm.so: src/qtplugin.o src/init.o src/config.o src/dlhook.o src/failsafe.o src/menu.o src/action_c.o src/action_cc.o
+src/libnm.so: src/qtplugin.o src/init.o src/config.o src/dlhook.o src/failsafe.o src/menu.o src/action.o src/action_c.o src/action_cc.o
 
 override LIBRARIES += src/libnm.so
 override MOCS      += src/qtplugin.moc

res/doc

@@ -21,6 +21,8 @@
 #     <action>    the type of action to run, one of:
 #                   dbg_syslog     - writes a message to syslog (for testing)
 #                   dbg_error      - always returns an error (for testing)
+#                   dbg_msg        - shows a message (for testing)
+#                   dbg_toast      - shows a toast (for testing)
 #                   kfmon          - triggers a kfmon action (TODO)
 #                   nickel_setting - toggles a boolean setting
 #                   nickel_extras  - opens one of the beta features
@@ -33,9 +35,9 @@
 #                   kfmon          - TODO
 #                   nickel_setting - one of:
 #                                      invert      - toggles FeatureSettings.InvertScreen (all versions)
-#                                      screenshots - toggles FeatureSettings.Screenshots (all versions) (TODO)
+#                                      screenshots - toggles FeatureSettings.Screenshots (all versions)
 #                   nickel_extras - the mimetype of the plugin, or one of:
-#                                      web_browser (TODO)
+#                                      web_browser
 #                                      unblock_it
 #                                      sketch_pad
 #                                      solitaire

src/action.c

@@ -0,0 +1,37 @@
+#define _GNU_SOURCE // vasprintf
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "action.h"
+
+nm_action_result_t *nm_action_result_silent() {
+    nm_action_result_t *res = calloc(1, sizeof(nm_action_result_t));
+    res->type = NM_ACTION_RESULT_TYPE_SILENT;
+    return res;
+}
+
+#define _nm_action_result_fmt(_fn, _typ)                                 \
+    nm_action_result_t *nm_action_result_##_fn(const char *fmt, ...) { \
+        nm_action_result_t *res = calloc(1, sizeof(nm_action_result_t)); \
+        res->type = _typ;                                                \
+        va_list v;                                                       \
+        va_start(v, fmt);                                                \
+        if (vasprintf(&res->msg, fmt, v) == -1)                          \
+            res->msg = strdup("error");                                  \
+        va_end(v);                                                       \
+        return res;                                                      \
+    }
+
+_nm_action_result_fmt(msg,    NM_ACTION_RESULT_TYPE_MSG);
+_nm_action_result_fmt(toast,  NM_ACTION_RESULT_TYPE_TOAST);
+
+void nm_action_result_free(nm_action_result_t *res) {
+    if (!res)
+        return;
+
+    free(res);
+    if (res->msg)
+        free(res->msg);
+}

src/action.h

@@ -0,0 +1,52 @@
+#ifndef NM_ACTION_H
+#define NM_ACTION_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef enum {
+    NM_ACTION_RESULT_TYPE_SILENT = 0,
+    NM_ACTION_RESULT_TYPE_MSG    = 1,
+    NM_ACTION_RESULT_TYPE_TOAST  = 2,
+} nm_action_result_type_t;
+
+typedef struct {
+    nm_action_result_type_t type;
+    char *msg;
+} nm_action_result_t;
+
+typedef nm_action_result_t *(*nm_action_fn_t)(const char *arg, char **err);
+
+nm_action_result_t *nm_action_result_silent();
+nm_action_result_t *nm_action_result_msg(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+nm_action_result_t *nm_action_result_toast(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+void nm_action_result_free(nm_action_result_t *res);
+
+#define NM_ACTION(name) nm_action_##name
+
+#ifdef __cplusplus
+#define NM_ACTION_(name) extern "C" nm_action_result_t *NM_ACTION(name)(const char *arg, char **err_out)
+#else
+#define NM_ACTION_(name) nm_action_result_t *NM_ACTION(name)(const char *arg, char **err_out)
+#endif
+
+#define NM_ACTIONS    \
+    X(dbg_syslog)     \
+    X(dbg_error)      \
+    X(dbg_msg)        \
+    X(dbg_toast)      \
+    X(kfmon)          \
+    X(nickel_setting) \
+    X(nickel_extras)  \
+    X(nickel_misc)    \
+    X(cmd_spawn)      \
+    X(cmd_output)
+
+#define X(name) NM_ACTION_(name);
+NM_ACTIONS
+#undef X
+
+#ifdef __cplusplus
+}
+#endif
+#endif

src/action_c.c

@@ -1,23 +1,35 @@
 #define _GNU_SOURCE // asprintf
 
-#include "action_c.h"
+#include "action.h"
 #include "util.h"
 
-int nm_action_dbgsyslog(const char *arg, char **err_out) {
+NM_ACTION_(dbg_syslog) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET NULL
     NM_LOG("dbgsyslog: %s", arg);
-    NM_RETURN_OK(0);
+    NM_RETURN_OK(nm_action_result_silent());
     #undef NM_ERR_RET
 }
 
-int nm_action_dbgerror(const char *arg, char **err_out) {
+NM_ACTION_(dbg_error) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET NULL
     NM_RETURN_ERR("%s", arg);
     #undef NM_ERR_RET
 }
 
-int nm_action_kfmon(const char *arg, char **err_out) {
+NM_ACTION_(dbg_msg) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET NULL
+    NM_RETURN_OK(nm_action_result_msg("%s", arg));
+    #undef NM_ERR_RET
+}
+
+NM_ACTION_(dbg_toast) {
+    #define NM_ERR_RET NULL
+    NM_RETURN_OK(nm_action_result_toast("%s", arg));
+    #undef NM_ERR_RET
+}
+
+NM_ACTION_(kfmon) {
+    #define NM_ERR_RET NULL
     NM_RETURN_ERR("not implemented yet (arg=%s)", arg); // TODO
     #undef NM_ERR_RET
 }

src/action_c.h

@@ -1,14 +0,0 @@
-#ifndef NM_ACTION_C_H
-#define NM_ACTION_C_H
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-int nm_action_dbgsyslog(const char *arg, char **err_out);
-int nm_action_dbgerror(const char *arg, char **err_out);
-int nm_action_kfmon(const char *arg, char **err_out);
-
-#ifdef __cplusplus
-}
-#endif
-#endif

src/action_cc.cc

@@ -17,7 +17,7 @@
 #include <string.h>
 #include <unistd.h>
 
-#include "action_cc.h"
+#include "action.h"
 #include "util.h"
 
 typedef void Device;
@@ -25,33 +25,9 @@ typedef void Settings;
 typedef void PlugWorkflowManager;
 typedef void BrowserWorkflowManager;
 typedef void N3SettingsExtrasController;
-typedef void MainWindowController;
 
-static void toast(QString const& primaryText, QString const& secondaryText, int msecs) {
+NM_ACTION_(nickel_setting) {
-    MainWindowController *(*MainWindowController_sharedInstance)();
+    #define NM_ERR_RET nullptr
-    reinterpret_cast<void*&>(MainWindowController_sharedInstance) = dlsym(RTLD_DEFAULT, "_ZN20MainWindowController14sharedInstanceEv");
-    if (!MainWindowController_sharedInstance) {
-        NM_LOG("toast: could not dlsym MainWindowController::sharedInstance");
-        return;
-    }
-
-    void (*MainWindowController_toast)(MainWindowController*, QString const&, QString const&, int);
-    reinterpret_cast<void*&>(MainWindowController_toast) = dlsym(RTLD_DEFAULT, "_ZN20MainWindowController5toastERK7QStringS2_i");
-    if (!MainWindowController_toast) {
-        NM_LOG("toast: could not dlsym MainWindowController::toast");
-        return;
-    }
-
-    MainWindowController *mwc = MainWindowController_sharedInstance();
-    if (!mwc) {
-        NM_LOG("toast: could not get shared main window controller pointer");
-        return;
-    }
-    MainWindowController_toast(mwc, primaryText, secondaryText, msecs);
-}
-
-extern "C" int nm_action_nickelsetting(const char *arg, char **err_out) {
-    #define NM_ERR_RET 1
 
     Device *(*Device_getCurrentDevice)();
     reinterpret_cast<void*&>(Device_getCurrentDevice) = dlsym(RTLD_DEFAULT, "_ZN6Device16getCurrentDeviceEv");
@@ -137,15 +113,14 @@ extern "C" int nm_action_nickelsetting(const char *arg, char **err_out) {
 
     Settings_SettingsD(settings);
 
-    if (strcmp(arg, "invert")) // invert is obvious
+    NM_RETURN_OK(strcmp(arg, "invert") // invert is obvious
-        toast(arg, v ? QStringLiteral("Disabled") : QStringLiteral("Enabled"), 1500);
+        ? nm_action_result_toast("%s %s", v ? "disabled" : "enabled", arg)
-
+        : nm_action_result_silent());
-    NM_RETURN_OK(0);
     #undef NM_ERR_RET
 }
 
-extern "C" int nm_action_nickelextras(const char *arg, char **err_out) {
+NM_ACTION_(nickel_extras) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET nullptr
 
     if (!strcmp(arg, "web_browser")) {
         void (*N3SettingsExtrasController_N3SettingsExtrasController)(N3SettingsExtrasController*);
@@ -177,7 +152,7 @@ extern "C" int nm_action_nickelextras(const char *arg, char **err_out) {
         BrowserWorkflowManager *bwm = alloca(128); // as of 4.20.14622, it's actually 20 bytes, but we're going to stay on the safe side
         BrowserWorkflowManager_BrowserWorkflowManager(bwm, new QObject());
         BrowserWorkflowManager_openBrowser(bwm, false, QUrl(), QStringLiteral("")); // if !QUrl::isValid(), it loads the homepage*/
-        NM_RETURN_OK(0);
+        NM_RETURN_OK(nm_action_result_silent());
     }
 
     const char* mimetype;
@@ -194,12 +169,12 @@ extern "C" int nm_action_nickelextras(const char *arg, char **err_out) {
     NM_ASSERT(ExtrasPluginLoader_loadPlugin, "could not dlsym ExtrasPluginLoader::loadPlugin");
     ExtrasPluginLoader_loadPlugin(mimetype);
 
-    NM_RETURN_OK(0);
+    NM_RETURN_OK(nm_action_result_silent());
     #undef NM_ERR_RET
 }
 
-extern "C" int nm_action_nickelmisc(const char *arg, char **err_out) {
+NM_ACTION_(nickel_misc) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET nullptr
     if (!strcmp(arg, "rescan_books")) {
         PlugWorkflowManager *(*PlugWorkflowManager_sharedInstance)();
         reinterpret_cast<void*&>(PlugWorkflowManager_sharedInstance) = dlsym(RTLD_DEFAULT, "_ZN19PlugWorkflowManager14sharedInstanceEv");
@@ -245,17 +220,12 @@ extern "C" int nm_action_nickelmisc(const char *arg, char **err_out) {
     } else {
         NM_RETURN_ERR("unknown action '%s'", arg);
     }
-    NM_RETURN_OK(0);
+    NM_RETURN_OK(nm_action_result_silent());
     #undef NM_ERR_RET
 }
 
-// TODO: stop abusing err_out to display messages, maybe add a msg_out arg
+NM_ACTION_(cmd_spawn) {
-// for that kind of thing instead (I've marked those spots by returning 2 for
+    #define NM_ERR_RET nullptr
-// now).
-
-extern "C" int nm_action_cmdspawn(const char *arg, char **err_out) {
-    #define NM_ERR_RET 1
-
     QProcess proc;
     uint64_t pid;
     bool ok = proc.startDetached(
@@ -268,16 +238,12 @@ extern "C" int nm_action_cmdspawn(const char *arg, char **err_out) {
         (qint64*)(&pid)
     );
     NM_ASSERT(ok, "could not start process");
-
+    NM_RETURN_OK(nm_action_result_toast("Successfully started process with PID %lu.", (unsigned long)(pid)));
-    if (*err_out)
-        asprintf(err_out, "Successfully started process with PID %lu.", (unsigned long)(pid));
-    return 2;
-
     #undef NM_ERR_RET
 }
 
-extern "C" int nm_action_cmdoutput(const char *arg, char **err_out) {
+NM_ACTION_(cmd_output) {
-    #define NM_ERR_RET 1
+    #define NM_ERR_RET nullptr
 
     char *tmp = strdup(arg);
 
@@ -320,9 +286,7 @@ extern "C" int nm_action_cmdoutput(const char *arg, char **err_out) {
 
     free(tmp);
 
-    if (err_out)
+    NM_RETURN_OK(nm_action_result_msg("%s", qPrintable(out)));
-        *err_out = strdup(qPrintable(out));
-    return 2;
 
     #undef NM_ERR_RET
 }

src/action_cc.h

@@ -1,19 +0,0 @@
-#ifndef NM_ACTION_CC_H
-#define NM_ACTION_CC_H
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-// This file contains actions which need access to Qt classes, but is still
-// mostly C code.
-
-int nm_action_nickelsetting(const char *arg, char **err_out);
-int nm_action_nickelextras(const char *arg, char **err_out);
-int nm_action_nickelmisc(const char *arg, char **err_out);
-int nm_action_cmdspawn(const char *arg, char **err_out);
-int nm_action_cmdoutput(const char *arg, char **err_out);
-
-#ifdef __cplusplus
-}
-#endif
-#endif

src/config.c

@@ -10,8 +10,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#include "action_c.h"
+#include "action.h"
-#include "action_cc.h"
 #include "config.h"
 #include "menu.h"
 #include "util.h"
@@ -130,14 +129,9 @@ nm_config_t *nm_config_parse(char **err_out) {
                 // type: menu_item - field 4: action
                 char *c_act = strtrim(strsep(&cur, ":"));
                 if (!c_act) RETERR("file %s: line %d: field 4: expected action, got end of line", fn, line_n);
-                else if (!strcmp(c_act, "dbg_syslog"))     it->act = nm_action_dbgsyslog;
+                #define X(name) else if (!strcmp(c_act, #name)) it->act = NM_ACTION(name);
-                else if (!strcmp(c_act, "dbg_error"))      it->act = nm_action_dbgerror;
+                NM_ACTIONS
-                else if (!strcmp(c_act, "kfmon"))          it->act = nm_action_kfmon;
+                #undef X
-                else if (!strcmp(c_act, "nickel_setting")) it->act = nm_action_nickelsetting;
-                else if (!strcmp(c_act, "nickel_extras"))  it->act = nm_action_nickelextras;
-                else if (!strcmp(c_act, "nickel_misc"))    it->act = nm_action_nickelmisc;
-                else if (!strcmp(c_act, "cmd_spawn"))      it->act = nm_action_cmdspawn;
-                else if (!strcmp(c_act, "cmd_output"))     it->act = nm_action_cmdoutput;
                 else RETERR("file %s: line %d: field 4: unknown action '%s'", fn, line_n, c_act);
 
                 // type: menu_item - field 5: argument
@@ -165,7 +159,7 @@ nm_config_t *nm_config_parse(char **err_out) {
         it->loc = NM_MENU_LOCATION_MAIN_MENU;
         it->lbl = strdup("NickelMenu");
         it->arg = strdup("See KOBOeReader/.add/nm/doc for instructions on how to customize this menu.");
-        it->act = nm_action_dbgerror;
+        it->act = NM_ACTION(dbg_toast);
         nm_config_push_menu_item(&cfg, it);
     }
 

src/init.c

@@ -7,8 +7,7 @@
 #include <string.h>
 #include <unistd.h>
 
-#include "action_c.h"
+#include "action.h"
-#include "action_cc.h"
 #include "config.h"
 #include "failsafe.h"
 #include "menu.h"
@@ -52,7 +51,7 @@ __attribute__((constructor)) void nm_init() {
         items[0]->loc = NM_MENU_LOCATION_MAIN_MENU;
         items[0]->lbl = strdup("Config Error");
         items[0]->arg = strdup(err);
-        items[0]->act = nm_action_dbgerror;
+        items[0]->act = NM_ACTION(dbg_msg);
 
         free(err);
     } else if (!(items = nm_config_get_menu(cfg, &items_n))) {

src/menu.cc

@@ -7,11 +7,13 @@
 #include <cstdlib>
 #include <dlfcn.h>
 
+#include "action.h"
 #include "dlhook.h"
 #include "menu.h"
 #include "util.h"
 
 typedef QWidget MenuTextItem; // it's actually a subclass, but we don't need it's functionality directly, so we'll stay on the safe side
+typedef void MainWindowController;
 
 // AbstractNickelMenuController::createMenuTextItem creates a menu item in the
 // given QMenu with the specified label. The first bool parameter adds space to
@@ -33,6 +35,13 @@ static QAction* (*AbstractNickelMenuController_createAction)(void*, QMenu*, QWid
 // a signal handler).
 static void (*ConfirmationDialogFactory_showOKDialog)(QString const&, QString const&);
 
+MainWindowController *(*MainWindowController_sharedInstance)();
+
+// MainWindowController::toast shows a message (primary and secondary text) as
+// an overlay for a number of milliseconds. It should also be called from the
+// GUI thread.
+void (*MainWindowController_toast)(MainWindowController*, QString const&, QString const&, int);
+
 static nm_menu_item_t **_items;
 static size_t _items_n;
 
@@ -41,10 +50,14 @@ extern "C" int nm_menu_hook(void *libnickel, nm_menu_item_t **items, size_t item
     reinterpret_cast<void*&>(AbstractNickelMenuController_createMenuTextItem) = dlsym(libnickel, "_ZN28AbstractNickelMenuController18createMenuTextItemEP5QMenuRK7QStringbbS4_");
     reinterpret_cast<void*&>(AbstractNickelMenuController_createAction) = dlsym(libnickel, "_ZN22AbstractMenuController12createActionEP5QMenuP7QWidgetbbb");
     reinterpret_cast<void*&>(ConfirmationDialogFactory_showOKDialog) = dlsym(libnickel, "_ZN25ConfirmationDialogFactory12showOKDialogERK7QStringS2_");
+    reinterpret_cast<void*&>(MainWindowController_sharedInstance) = dlsym(libnickel, "_ZN20MainWindowController14sharedInstanceEv");
+    reinterpret_cast<void*&>(MainWindowController_toast) = dlsym(libnickel, "_ZN20MainWindowController5toastERK7QStringS2_i");
 
     NM_ASSERT(AbstractNickelMenuController_createMenuTextItem, "unsupported firmware: could not find AbstractNickelMenuController::createMenuTextItem(void* _this, QMenu*, QString, bool, bool, QString const&)");
     NM_ASSERT(AbstractNickelMenuController_createAction, "unsupported firmware: could not find AbstractNickelMenuController::createAction(void* _this, QMenu*, QWidget*, bool, bool, bool)");
-    NM_ASSERT(AbstractNickelMenuController_createAction, "unsupported firmware: could not find ConfirmationDialogFactory::showOKDialog(QString const&, QString const&)");
+    NM_ASSERT(ConfirmationDialogFactory_showOKDialog, "unsupported firmware: could not find ConfirmationDialogFactory::showOKDialog(String const&, QString const&)");
+    NM_ASSERT(MainWindowController_sharedInstance, "unsupported firmware: could not find MainWindowController::sharedInstance()");
+    NM_ASSERT(MainWindowController_toast, "unsupported firmware: could not find MainWindowController_toast(QString const&, QString const&, int)");
 
     void* nmh = dlsym(RTLD_DEFAULT, "_nm_menu_hook");
     NM_ASSERT(nmh, "internal error: could not dlsym _nm_menu_hook");
@@ -89,11 +102,33 @@ extern "C" MenuTextItem* _nm_menu_hook(void* _this, QMenu* menu, QString const&
         QObject::connect(action, &QAction::triggered, std::function<void(bool)>([it](bool){
             NM_LOG("Item '%s' pressed...", it->lbl);
             char *err;
-            if (it->act(it->arg, &err) && err) {
+            nm_action_result_t *res = it->act(it->arg, &err);
+            if (err) {
                 NM_LOG("Got error: '%s', displaying...", err);
                 ConfirmationDialogFactory_showOKDialog(QString::fromUtf8(it->lbl), QString::fromUtf8(err));
                 free(err);
                 return;
+            } else if (res) {
+                NM_LOG("Got result: type=%d msg='%s', handling...", res->type, res->msg);
+                MainWindowController *mwc;
+                switch (res->type) {
+                case NM_ACTION_RESULT_TYPE_SILENT:
+                    break;
+                case NM_ACTION_RESULT_TYPE_MSG:
+                    ConfirmationDialogFactory_showOKDialog(QString::fromUtf8(it->lbl), QLatin1String(res->msg));
+                    break;
+                case NM_ACTION_RESULT_TYPE_TOAST:
+                    mwc = MainWindowController_sharedInstance();
+                    if (!mwc) {
+                        NM_LOG("toast: could not get shared main window controller pointer");
+                        break;
+                    }
+                    MainWindowController_toast(mwc, QLatin1String(res->msg), QStringLiteral(""), 1500);
+                    break;
+                }
+                nm_action_result_free(res);
+            } else {
+                NM_LOG("warning: you should have returned a result with type silent, not null, upon success");
             }
             NM_LOG("Success!");
         }));

src/menu.h

@@ -6,6 +6,8 @@ extern "C" {
 
 #include <stddef.h>
 
+#include "action.h"
+
 typedef enum {
     NM_MENU_LOCATION_MAIN_MENU   = 1,
     NM_MENU_LOCATION_READER_MENU = 2,
@@ -15,7 +17,7 @@ typedef struct {
     nm_menu_location_t loc;
     char *lbl;
     char *arg;
-    int (*act)(const char *arg, char **out_err); // can block, must return 0 on success, nonzero with out_err set to the malloc'd error message on error
+    nm_action_fn_t act; // can block, must return 0 on success, nonzero with out_err set to the malloc'd error message on error
 } nm_menu_item_t;
 
 // nm_menu_hook hooks a dlopen'd libnickel handle to add the specified menus,