[PATCH v3 0/5] Fix Bug#12935 + cosmetic changes/enhancements

[PATCH v3 0/5] Fix Bug#12935 + cosmetic changes/enhancements

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]

Hi all

After carefully reviewing and adopting all comments Michael had about
the code in previous patchset, here is again a new version of it.

Only PATCH 1 was changed since v2:
- propper initialization of variables where required preventing possible
  undefined behaviour
- more reliable error checking: all functions now return a meaningfull
  integer one way or another to indicate if and what kind of error
  happend. No need anymore to set errno to 0 manually as it no longer
  depended on.
- No more checking against NULL making many comparisions easier for the
  eye
- fix some possible memory leaks
- fix a possible unallocation of not yet allocated memory
- in case of a system error, a descriptive error is shown instead of
  the number, using the %m directive.

Hoping to pass the required strict evaluation this time :-).

For reference, the content of the summary mail that was sent with v1 of
the patch:
---
This patchset fixes Bug#12935
(https://bugzilla.ipfire.org/show_bug.cgi?id=12935)

Summary:
Addons where the initscript does not match the addon-name and addons with
multiple initscripts are now listed on services.cgi since CU170.
But addonctrl still expected addon name to be equal to
initscript name; Hence starting/stopping/enabling/disabling of such
addons was not possible.
This has always been like that, but that problem was hidden as
services.cgi also did not display those addon services.

After discussing this with Adolf on the Bug report, we concluded that we
should adapt addonctrl to work with the new addon metadata
Services-field instead.

I basically rewrote addonctrl to not only use the new services metadata
but also to have better errorchecking and added the posibility to check
if a service is currently enabled or disabled.
As a result services.cgi no longer has to go checking the precense of
runlevel initscripts, but can just ask addonctrl.
I also added a warning to services.cgi if a runlevel initscript does not
exists, to prevent the user from wondering why he can't enable a
specific service. (Adolf pointed out some services don't install
runlevel initscripts by default)

More details in the bugreport and in the commit-messages of the patches.

Regards
Robin



-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

[PATCH v3 1/5] misc-progs: addonctrl: Add support for 'Services' metadata

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 16416 bytes --]

* Addonctrl will now check in addon metadata for the exact initscript
  names (Services). If more than one initscript is defined for an addon,
  the requested action will be performed on all listed initscripts.
* Added posibility to perform action on a specific initscript of an
  addon instead of on all initscripts of the addon.
* New action 'list-services' to display a list of services related to
  an addon.
* New action 'boot-status' to display wether service(s) are enabled
  to start on boot or not.
* More error checking and cleaner error reporting to user
* General cleanup and code restructuring to avoid code duplication
* Updated and made usage instructions more verbose.

Fixes: Bug#12935
Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
---
 src/misc-progs/addonctrl.c | 459 ++++++++++++++++++++++++++++++++-----
 1 file changed, 398 insertions(+), 61 deletions(-)

diff --git a/src/misc-progs/addonctrl.c b/src/misc-progs/addonctrl.c
index 14b4b1325..8eb7fbfa5 100644
--- a/src/misc-progs/addonctrl.c
+++ b/src/misc-progs/addonctrl.c
@@ -10,71 +10,408 @@
 #include <string.h>
 #include <unistd.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <fcntl.h>
+#include <dirent.h>
+#include <fnmatch.h>
+#include <errno.h>
 #include "setuid.h"
 
 #define BUFFER_SIZE 1024
 
+const char *initd_path = "/etc/rc.d/init.d";
+const char *enabled_path = "/etc/rc.d/rc3.d";
+const char *disabled_path = "/etc/rc.d/rc3.d/off";
+
+const char *usage = 
+    "Usage\n"
+    "  addonctrl <addon> (start|stop|restart|reload|enable|disable|status|boot-status|list-services) [<service>]\n"
+    "\n"
+    "Options:\n"
+    "  <addon>\t\tName of the addon to control\n"
+    "  <service>\t\tSpecific service of the addon to control (optional)\n"
+    "  \t\t\tBy default the requested action is performed on all related services. See also 'list-services'.\n"
+    "  start\t\t\tStart service(s) of the addon\n"
+    "  stop\t\t\tStop service(s) of the addon\n"
+    "  restart\t\tRestart service(s) of the addon\n"
+    "  enable\t\tEnable service(s) of the addon to start at boot\n"
+    "  disable\t\tDisable service(s) of the addon to start at boot\n"
+    "  status\t\tDisplay current state of addon service(s)\n"
+    "  boot-status\t\tDisplay wether service(s) is enabled on boot or not\n"
+    "  list-services\t\tDisplay a list of services related to the addon";
+
+// Find a file <path> using <filepattern> as glob pattern. 
+// Returns the found filename or NULL if not found
+char *find_file_in_dir(const char *path, const char *filepattern) 
+{
+    struct dirent *entry;
+    DIR *dp;
+    char *found = NULL;
+
+    dp = opendir(path);
+    if (dp) {
+        entry = readdir(dp);
+        while(!found && entry) {
+            if (fnmatch(filepattern, entry->d_name, FNM_PATHNAME) == 0)
+                found = strdup(entry->d_name);
+            else
+                entry = readdir(dp);
+        }
+
+        closedir(dp);
+    }
+
+    return found;
+}
+
+// Reads Services metadata for <addon>.
+// Returns pointer to array of strings containing the services for <addon>,
+// sets <servicescnt> to the number of found services and 
+// sets <returncode> to 
+// -1 - system error occured, check errno
+// 0 - success - if returned array is NULL, there are no services for <addon>
+// 1 - addon was not found
+char **get_addon_services(const char *addon, int *servicescnt, const char *filter, int *returncode) {
+    const char *metafile_prefix = "/opt/pakfire/db/installed/meta-";
+    const char *metadata_key = "Services";
+    const char *keyvalue_delim = ":";
+    const char *service_delim = " ";
+    char *token;
+    char **services = NULL;
+    char *service;
+    char *line = NULL;
+    size_t line_len = 0;
+    int i = 0;
+    char *metafile = NULL;
+
+    *returncode = 0;
+ 
+    if (!addon) {
+        errno = EINVAL;
+        *returncode = 1;
+        return NULL;
+    }
+
+    *returncode = asprintf(&metafile, "%s%s", metafile_prefix, addon);
+    if (*returncode == -1)
+        return NULL;
+
+    FILE *fp = fopen(metafile,"r");
+    if (!fp) {
+        if (errno == ENOENT) {
+            *returncode = 1;
+        } else {
+            *returncode = -1;
+        }
+        return NULL;
+    }
+
+    // Get initscript(s) for addon from meta-file
+    while (getline(&line, &line_len, fp) != -1 && !services) {
+        // Strip newline
+        char *newline = strchr(line, '\n');
+        if (newline) *newline = 0;
+
+        // Split line in key and values; Check for required key.
+        token = strtok(line, keyvalue_delim);
+        if (!token || strcmp(token, metadata_key) != 0) 
+            continue;
+
+        // Get values for matched key. Stop if no values are present.
+        token = strtok(NULL, keyvalue_delim);
+        if (!token) 
+            break;
+
+        // Split values and put each service in services array
+        service = strtok(token, service_delim);
+        while (service) {
+            if (!filter || strcmp(filter, service) == 0) {
+                services = reallocarray(services, i+1 ,sizeof (char *));
+                if (!services) {
+                    *returncode = -1;
+                    break;
+                } 
+
+                services[i] = strdup(service);
+                if (!services[i++]) {
+                    *returncode = -1;
+                    break;
+                }
+            }
+
+            service = strtok(NULL, service_delim);
+        }
+    }
+
+    if (line) free(line);
+    fclose(fp);
+    free(metafile);
+
+    *servicescnt = i;
+
+    return services;
+}
+
+// Calls initscript <service> with parameter <action>
+int initscript_action(const char *service, const char *action) {
+    char *initscript = NULL;
+    char *argv[] = {
+        action,
+        NULL
+    };
+    int r = 0;
+
+    r = asprintf(&initscript, "%s/%s", initd_path, service);
+    if (r != -1)
+        r = run(initscript, argv);
+
+    if (initscript) free(initscript);
+
+    return r; 
+}
+
+// Move an initscript with filepattern from <src_path> to <dest_path>
+// Returns:
+//   -1: Error during move or memory allocation. Details in errno
+//   0: Success
+//   1: file was not moved, but is already in <dest_path>
+//   2: file does not exist in either in <src_path> or <dest_path>
+int move_initscript_by_pattern(const char *src_path, const char *dest_path, const char *filepattern) {
+    char *src = NULL;
+    char *dest = NULL;
+    int r = 2;
+    char *filename = NULL;
+
+    filename = find_file_in_dir(src_path, filepattern);
+    if (filename) {
+        // Move file
+        r = asprintf(&src, "%s/%s", src_path, filename);
+        if (r != -1) {
+            r = asprintf(&dest, "%s/%s", dest_path, filename);
+            if (r != -1)
+                r = rename(src, dest);
+        }
+
+        if (src) free(src);
+        if (dest) free(dest);
+    } else {
+        // check if file is already in dest
+        filename = find_file_in_dir(dest_path, filepattern);
+        if (filename) 
+            r = 1;
+    }
+
+    if (filename) free(filename);
+
+    return r;
+}
+
+// Enable/Disable addon service(s) by moving initscript symlink from/to disabled_path
+// Returns:
+// -1 - System error occured. Check errno.
+// 0 - Success
+// 1 - Service was already enabled/disabled
+// 2 - Service has no valid runlevel symlink
+int toggle_service(const char *service, const char *action) {
+    const char *src_path, *dest_path;
+    char *filepattern = NULL; 
+    int r = 0;
+    
+    if (asprintf(&filepattern, "S??%s", service) == -1)
+        return -1;
+
+    if (strcmp(action, "enable") == 0) {
+        src_path = disabled_path;
+        dest_path = enabled_path;
+    } else {
+        src_path = enabled_path;
+        dest_path = disabled_path;
+    }
+
+    // Ensure disabled_path exists
+    r = mkdir(disabled_path, S_IRWXU + S_IRGRP + S_IXGRP + S_IROTH + S_IXOTH); 
+    if (r != -1 || errno == EEXIST)
+        r = move_initscript_by_pattern(src_path, dest_path, filepattern);
+
+    free(filepattern);
+    
+    return r;
+}
+
+// Return whether <service> is enabled or disabled on boot
+// Returns:
+// -1 - System error occured. Check errno.
+// 0 - <service> is disabled on boot
+// 1 - <service> is enabled on boot
+// 2 - Runlevel suymlink for <service> was not found
+int get_boot_status(char *service) {
+    char *filepattern = NULL;
+    char *filename = NULL;
+    int r = 2;
+
+    if (asprintf(&filepattern, "S??%s", service) == -1)
+        return -1;
+
+    filename = find_file_in_dir(enabled_path, filepattern);
+    if (filename)
+        r = 1;
+    else {
+        filename = find_file_in_dir(disabled_path, filepattern);
+        if (filename)
+            r = 0;
+        else
+            r = 2;
+    }
+
+    if (filename) free(filename);
+    free(filepattern);
+
+    return r;
+}
+
 int main(int argc, char *argv[]) {
-	char command[BUFFER_SIZE];
-
-	if (!(initsetuid()))
-		exit(1);
-
-	if (argc < 3) {
-		fprintf(stderr, "\nMissing arguments.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
-		exit(1);
-	}
-
-	const char* name = argv[1];
-
-	if (strlen(name) > 32) {
-	    fprintf(stderr, "\nString to large.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
-	    exit(1);
-	}
-
-	// Check if the input argument is valid
-	if (!is_valid_argument_alnum(name)) {
-		fprintf(stderr, "Invalid add-on name: %s\n", name);
-		exit(2);
-	}
-
-	sprintf(command, "/opt/pakfire/db/installed/meta-%s", name);
-	FILE *fp = fopen(command,"r");
-	if ( fp ) {
-	    fclose(fp);
-	} else {
-	    fprintf(stderr, "\nAddon '%s' not found.\n\naddonctrl addon (start|stop|restart|reload|status|enable|disable)\n\n", name);
-	    exit(1);
-	}
-
-	if (strcmp(argv[2], "start") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s start", name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "stop") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s stop", name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "restart") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s restart", name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "reload") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s reload", name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "status") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s status", name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "enable") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "mv -f /etc/rc.d/rc3.d/off/S??%s /etc/rc.d/rc3.d" , name);
-		safe_system(command);
-	} else if (strcmp(argv[2], "disable") == 0) {
-		snprintf(command, BUFFER_SIZE - 1, "mkdir -p /etc/rc.d/rc3.d/off");
-		safe_system(command);
-		snprintf(command, BUFFER_SIZE - 1, "mv -f /etc/rc.d/rc3.d/S??%s /etc/rc.d/rc3.d/off" , name);
-		safe_system(command);
-	} else {
-		fprintf(stderr, "\nBad argument given.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
-		exit(1);
-	}
-
-	return 0;
+    char **services = NULL;
+    int servicescnt = 0;
+    char *addon = argv[1];
+    char *action = argv[2];
+    char *service_filter = NULL;
+    int r = 0;
+
+    if (!(initsetuid()))
+        exit(1);
+
+    if (argc < 3) {
+        fprintf(stderr, "\nMissing arguments.\n\n%s\n\n", usage);
+        exit(1);
+    }
+
+    // Ignore filter when list of services is requested
+    if (argc == 4 && strcmp(action, "list-services") != 0)
+        service_filter = argv[3];
+
+    if (strlen(addon) > 32) {
+        fprintf(stderr, "\nString too large.\n\n%s\n\n", usage);
+        exit(1);
+    }
+
+    // Check if the input argument is valid
+    if (!is_valid_argument_alnum(addon)) {
+        fprintf(stderr, "Invalid add-on name: %s.\n", addon);
+        exit(2);
+    }
+
+    // Get initscript name(s) from addon metadata
+    int rc = 0;
+    services = get_addon_services(addon, &servicescnt, service_filter, &rc);
+    if (!services) {
+        switch (rc) {
+            case -1:
+                fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
+                break;
+
+            case 0:
+                if (service_filter)
+                    fprintf(stderr, "\nNo service '%s' found for addon '%s'. Use 'list-services' to get a list of available services\n\n%s\n\n", service_filter, addon, usage);
+                else
+                    fprintf(stderr, "\nAddon '%s' has no services.\n\n", addon);
+                break;
+
+            case 1:
+                fprintf(stderr, "\nAddon '%s' not found.\n\n%s\n\n", addon, usage);
+                break;
+        }
+        exit(1);
+    }
+        
+    // Handle requested action
+    if (strcmp(action, "start") == 0 ||
+        strcmp(action, "stop") == 0 ||
+        strcmp(action, "restart") == 0 ||
+        strcmp(action, "reload") == 0 ||
+        strcmp(action, "status") == 0) {
+
+        for(int i = 0; i < servicescnt; i++) {
+            if (initscript_action(services[i], action) < 0) {
+                r = 1;
+                fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
+                break;
+            }
+        }
+
+    } else if (strcmp(action, "enable") == 0 ||
+               strcmp(action, "disable") == 0) {
+
+        for(int i = 0; i < servicescnt; i++) {
+            switch (r = toggle_service(services[i], action)) {
+                case -1:
+                    fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
+                    break;
+
+                case 0:
+                    printf("%sd service %s\n", action, services[i]);
+                    break;
+                
+                case 1:
+                    fprintf(stderr, "Service %s is already %sd. Skipping...\n", services[i], action);
+                    break;
+
+                case 2:
+                    fprintf(stderr, "\nUnable to %s service %s. (Service has no valid runlevel symlink).\n\n", action, services[i]);
+                    break;
+            }
+
+            // Break from for loop in case of a system error.
+            if (r == -1) {
+                r = 1;
+                break;
+            }
+        }
+
+    } else if (strcmp(action, "boot-status") == 0) {
+        // Print boot status for each service
+        for(int i = 0; i < servicescnt; i++) {
+            switch (get_boot_status(services[i])) {
+                case -1:
+                    r = 1;
+                    fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
+                    break;
+                
+                case 0:
+                    printf("%s is disabled on boot.\n", services[i]);
+                    break;
+                
+                case 1:
+                    printf("%s is enabled on boot.\n", services[i]);
+                    break;
+
+                case 2:
+                    printf("%s is not available for boot. (Service has no valid symlink in either %s or %s).\n", services[i], enabled_path, disabled_path);
+                    break;
+            }
+            
+            // Break from for loop in case of an error
+            if (r == 1) {
+                break;
+            }
+        }
+    
+    } else if (strcmp(action, "list-services") == 0) {
+        // List all services for addon
+        printf("\nServices for addon %s:\n", addon);
+        for(int i = 0; i < servicescnt; i++) {
+            printf("  %s\n", services[i]);
+        }
+        printf("\n");
+
+    } else {
+        fprintf(stderr, "\nBad argument given.\n\n%s\n\n", usage);
+        r = 1;
+    }
+
+    // Cleanup
+    for(int i = 0; i < servicescnt; i++) 
+        free(services[i]);
+    free(services);
+
+    return r;
 }
-- 
2.37.3


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

Re: [PATCH v3 1/5] misc-progs: addonctrl: Add support for 'Services' metadata

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 17548 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> * Addonctrl will now check in addon metadata for the exact initscript
>  names (Services). If more than one initscript is defined for an addon,
>  the requested action will be performed on all listed initscripts.
> * Added posibility to perform action on a specific initscript of an
>  addon instead of on all initscripts of the addon.
> * New action 'list-services' to display a list of services related to
>  an addon.
> * New action 'boot-status' to display wether service(s) are enabled
>  to start on boot or not.
> * More error checking and cleaner error reporting to user
> * General cleanup and code restructuring to avoid code duplication
> * Updated and made usage instructions more verbose.
> 
> Fixes: Bug#12935
> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> ---
> src/misc-progs/addonctrl.c | 459 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 398 insertions(+), 61 deletions(-)
> 
> diff --git a/src/misc-progs/addonctrl.c b/src/misc-progs/addonctrl.c
> index 14b4b1325..8eb7fbfa5 100644
> --- a/src/misc-progs/addonctrl.c
> +++ b/src/misc-progs/addonctrl.c
> @@ -10,71 +10,408 @@
> #include <string.h>
> #include <unistd.h>
> #include <sys/types.h>
> +#include <sys/stat.h>
> #include <fcntl.h>
> +#include <dirent.h>
> +#include <fnmatch.h>
> +#include <errno.h>
> #include "setuid.h"
> 
> #define BUFFER_SIZE 1024
> 
> +const char *initd_path = "/etc/rc.d/init.d";
> +const char *enabled_path = "/etc/rc.d/rc3.d";
> +const char *disabled_path = "/etc/rc.d/rc3.d/off";
> +
> +const char *usage = 
> +    "Usage\n"
> +    "  addonctrl <addon> (start|stop|restart|reload|enable|disable|status|boot-status|list-services) [<service>]\n"
> +    "\n"
> +    "Options:\n"
> +    "  <addon>\t\tName of the addon to control\n"
> +    "  <service>\t\tSpecific service of the addon to control (optional)\n"
> +    "  \t\t\tBy default the requested action is performed on all related services. See also 'list-services'.\n"
> +    "  start\t\t\tStart service(s) of the addon\n"
> +    "  stop\t\t\tStop service(s) of the addon\n"
> +    "  restart\t\tRestart service(s) of the addon\n"
> +    "  enable\t\tEnable service(s) of the addon to start at boot\n"
> +    "  disable\t\tDisable service(s) of the addon to start at boot\n"
> +    "  status\t\tDisplay current state of addon service(s)\n"
> +    "  boot-status\t\tDisplay wether service(s) is enabled on boot or not\n"
> +    "  list-services\t\tDisplay a list of services related to the addon";
> +
> +// Find a file <path> using <filepattern> as glob pattern. 
> +// Returns the found filename or NULL if not found
> +char *find_file_in_dir(const char *path, const char *filepattern) 
> +{
> +    struct dirent *entry;
> +    DIR *dp;
> +    char *found = NULL;
> +
> +    dp = opendir(path);
> +    if (dp) {
> +        entry = readdir(dp);
> +        while(!found && entry) {
> +            if (fnmatch(filepattern, entry->d_name, FNM_PATHNAME) == 0)
> +                found = strdup(entry->d_name);
> +            else
> +                entry = readdir(dp);
> +        }
> +
> +        closedir(dp);
> +    }
> +
> +    return found;
> +}
> +
> +// Reads Services metadata for <addon>.
> +// Returns pointer to array of strings containing the services for <addon>,
> +// sets <servicescnt> to the number of found services and 
> +// sets <returncode> to 
> +// -1 - system error occured, check errno
> +// 0 - success - if returned array is NULL, there are no services for <addon>
> +// 1 - addon was not found
> +char **get_addon_services(const char *addon, int *servicescnt, const char *filter, int *returncode) {
> +    const char *metafile_prefix = "/opt/pakfire/db/installed/meta-";
> +    const char *metadata_key = "Services";
> +    const char *keyvalue_delim = ":";
> +    const char *service_delim = " ";
> +    char *token;
> +    char **services = NULL;
> +    char *service;
> +    char *line = NULL;
> +    size_t line_len = 0;
> +    int i = 0;
> +    char *metafile = NULL;
> +
> +    *returncode = 0;
> + 
> +    if (!addon) {
> +        errno = EINVAL;
> +        *returncode = 1;
> +        return NULL;
> +    }
> +
> +    *returncode = asprintf(&metafile, "%s%s", metafile_prefix, addon);
> +    if (*returncode == -1)
> +        return NULL;
> +
> +    FILE *fp = fopen(metafile,"r");
> +    if (!fp) {
> +        if (errno == ENOENT) {
> +            *returncode = 1;
> +        } else {
> +            *returncode = -1;
> +        }
> +        return NULL;
> +    }
> +
> +    // Get initscript(s) for addon from meta-file
> +    while (getline(&line, &line_len, fp) != -1 && !services) {
> +        // Strip newline
> +        char *newline = strchr(line, '\n');
> +        if (newline) *newline = 0;
> +
> +        // Split line in key and values; Check for required key.
> +        token = strtok(line, keyvalue_delim);
> +        if (!token || strcmp(token, metadata_key) != 0) 
> +            continue;
> +
> +        // Get values for matched key. Stop if no values are present.
> +        token = strtok(NULL, keyvalue_delim);
> +        if (!token) 
> +            break;
> +
> +        // Split values and put each service in services array
> +        service = strtok(token, service_delim);
> +        while (service) {
> +            if (!filter || strcmp(filter, service) == 0) {
> +                services = reallocarray(services, i+1 ,sizeof (char *));
> +                if (!services) {
> +                    *returncode = -1;
> +                    break;
> +                } 
> +
> +                services[i] = strdup(service);
> +                if (!services[i++]) {
> +                    *returncode = -1;
> +                    break;
> +                }
> +            }
> +
> +            service = strtok(NULL, service_delim);
> +        }
> +    }
> +
> +    if (line) free(line);
> +    fclose(fp);
> +    free(metafile);
> +
> +    *servicescnt = i;
> +
> +    return services;
> +}
> +
> +// Calls initscript <service> with parameter <action>
> +int initscript_action(const char *service, const char *action) {
> +    char *initscript = NULL;
> +    char *argv[] = {
> +        action,
> +        NULL
> +    };
> +    int r = 0;
> +
> +    r = asprintf(&initscript, "%s/%s", initd_path, service);
> +    if (r != -1)
> +        r = run(initscript, argv);
> +
> +    if (initscript) free(initscript);
> +
> +    return r; 
> +}
> +
> +// Move an initscript with filepattern from <src_path> to <dest_path>
> +// Returns:
> +//   -1: Error during move or memory allocation. Details in errno
> +//   0: Success
> +//   1: file was not moved, but is already in <dest_path>
> +//   2: file does not exist in either in <src_path> or <dest_path>
> +int move_initscript_by_pattern(const char *src_path, const char *dest_path, const char *filepattern) {
> +    char *src = NULL;
> +    char *dest = NULL;
> +    int r = 2;
> +    char *filename = NULL;
> +
> +    filename = find_file_in_dir(src_path, filepattern);
> +    if (filename) {
> +        // Move file
> +        r = asprintf(&src, "%s/%s", src_path, filename);
> +        if (r != -1) {
> +            r = asprintf(&dest, "%s/%s", dest_path, filename);
> +            if (r != -1)
> +                r = rename(src, dest);
> +        }
> +
> +        if (src) free(src);
> +        if (dest) free(dest);
> +    } else {
> +        // check if file is already in dest
> +        filename = find_file_in_dir(dest_path, filepattern);
> +        if (filename) 
> +            r = 1;
> +    }
> +
> +    if (filename) free(filename);
> +
> +    return r;
> +}
> +
> +// Enable/Disable addon service(s) by moving initscript symlink from/to disabled_path
> +// Returns:
> +// -1 - System error occured. Check errno.
> +// 0 - Success
> +// 1 - Service was already enabled/disabled
> +// 2 - Service has no valid runlevel symlink
> +int toggle_service(const char *service, const char *action) {
> +    const char *src_path, *dest_path;
> +    char *filepattern = NULL; 
> +    int r = 0;
> +    
> +    if (asprintf(&filepattern, "S??%s", service) == -1)
> +        return -1;
> +
> +    if (strcmp(action, "enable") == 0) {
> +        src_path = disabled_path;
> +        dest_path = enabled_path;
> +    } else {
> +        src_path = enabled_path;
> +        dest_path = disabled_path;
> +    }
> +
> +    // Ensure disabled_path exists
> +    r = mkdir(disabled_path, S_IRWXU + S_IRGRP + S_IXGRP + S_IROTH + S_IXOTH); 
> +    if (r != -1 || errno == EEXIST)
> +        r = move_initscript_by_pattern(src_path, dest_path, filepattern);
> +
> +    free(filepattern);
> +    
> +    return r;
> +}
> +
> +// Return whether <service> is enabled or disabled on boot
> +// Returns:
> +// -1 - System error occured. Check errno.
> +// 0 - <service> is disabled on boot
> +// 1 - <service> is enabled on boot
> +// 2 - Runlevel suymlink for <service> was not found
> +int get_boot_status(char *service) {
> +    char *filepattern = NULL;
> +    char *filename = NULL;
> +    int r = 2;
> +
> +    if (asprintf(&filepattern, "S??%s", service) == -1)
> +        return -1;
> +
> +    filename = find_file_in_dir(enabled_path, filepattern);
> +    if (filename)
> +        r = 1;
> +    else {
> +        filename = find_file_in_dir(disabled_path, filepattern);
> +        if (filename)
> +            r = 0;
> +        else
> +            r = 2;
> +    }
> +
> +    if (filename) free(filename);
> +    free(filepattern);
> +
> +    return r;
> +}
> +
> int main(int argc, char *argv[]) {
> -	char command[BUFFER_SIZE];
> -
> -	if (!(initsetuid()))
> -		exit(1);
> -
> -	if (argc < 3) {
> -		fprintf(stderr, "\nMissing arguments.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
> -		exit(1);
> -	}
> -
> -	const char* name = argv[1];
> -
> -	if (strlen(name) > 32) {
> -	    fprintf(stderr, "\nString to large.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
> -	    exit(1);
> -	}
> -
> -	// Check if the input argument is valid
> -	if (!is_valid_argument_alnum(name)) {
> -		fprintf(stderr, "Invalid add-on name: %s\n", name);
> -		exit(2);
> -	}
> -
> -	sprintf(command, "/opt/pakfire/db/installed/meta-%s", name);
> -	FILE *fp = fopen(command,"r");
> -	if ( fp ) {
> -	    fclose(fp);
> -	} else {
> -	    fprintf(stderr, "\nAddon '%s' not found.\n\naddonctrl addon (start|stop|restart|reload|status|enable|disable)\n\n", name);
> -	    exit(1);
> -	}
> -
> -	if (strcmp(argv[2], "start") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s start", name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "stop") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s stop", name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "restart") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s restart", name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "reload") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s reload", name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "status") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "/etc/rc.d/init.d/%s status", name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "enable") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "mv -f /etc/rc.d/rc3.d/off/S??%s /etc/rc.d/rc3.d" , name);
> -		safe_system(command);
> -	} else if (strcmp(argv[2], "disable") == 0) {
> -		snprintf(command, BUFFER_SIZE - 1, "mkdir -p /etc/rc.d/rc3.d/off");
> -		safe_system(command);
> -		snprintf(command, BUFFER_SIZE - 1, "mv -f /etc/rc.d/rc3.d/S??%s /etc/rc.d/rc3.d/off" , name);
> -		safe_system(command);
> -	} else {
> -		fprintf(stderr, "\nBad argument given.\n\naddonctrl addon (start|stop|restart|reload|enable|disable)\n\n");
> -		exit(1);
> -	}
> -
> -	return 0;
> +    char **services = NULL;
> +    int servicescnt = 0;
> +    char *addon = argv[1];
> +    char *action = argv[2];
> +    char *service_filter = NULL;
> +    int r = 0;
> +
> +    if (!(initsetuid()))
> +        exit(1);
> +
> +    if (argc < 3) {
> +        fprintf(stderr, "\nMissing arguments.\n\n%s\n\n", usage);
> +        exit(1);
> +    }
> +
> +    // Ignore filter when list of services is requested
> +    if (argc == 4 && strcmp(action, "list-services") != 0)
> +        service_filter = argv[3];
> +
> +    if (strlen(addon) > 32) {
> +        fprintf(stderr, "\nString too large.\n\n%s\n\n", usage);
> +        exit(1);
> +    }
> +
> +    // Check if the input argument is valid
> +    if (!is_valid_argument_alnum(addon)) {
> +        fprintf(stderr, "Invalid add-on name: %s.\n", addon);
> +        exit(2);
> +    }
> +
> +    // Get initscript name(s) from addon metadata
> +    int rc = 0;
> +    services = get_addon_services(addon, &servicescnt, service_filter, &rc);
> +    if (!services) {
> +        switch (rc) {
> +            case -1:
> +                fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
> +                break;
> +
> +            case 0:
> +                if (service_filter)
> +                    fprintf(stderr, "\nNo service '%s' found for addon '%s'. Use 'list-services' to get a list of available services\n\n%s\n\n", service_filter, addon, usage);
> +                else
> +                    fprintf(stderr, "\nAddon '%s' has no services.\n\n", addon);
> +                break;
> +
> +            case 1:
> +                fprintf(stderr, "\nAddon '%s' not found.\n\n%s\n\n", addon, usage);
> +                break;
> +        }
> +        exit(1);
> +    }
> +        
> +    // Handle requested action
> +    if (strcmp(action, "start") == 0 ||
> +        strcmp(action, "stop") == 0 ||
> +        strcmp(action, "restart") == 0 ||
> +        strcmp(action, "reload") == 0 ||
> +        strcmp(action, "status") == 0) {
> +
> +        for(int i = 0; i < servicescnt; i++) {
> +            if (initscript_action(services[i], action) < 0) {
> +                r = 1;
> +                fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
> +                break;
> +            }
> +        }
> +
> +    } else if (strcmp(action, "enable") == 0 ||
> +               strcmp(action, "disable") == 0) {
> +
> +        for(int i = 0; i < servicescnt; i++) {
> +            switch (r = toggle_service(services[i], action)) {
> +                case -1:
> +                    fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
> +                    break;
> +
> +                case 0:
> +                    printf("%sd service %s\n", action, services[i]);
> +                    break;
> +                
> +                case 1:
> +                    fprintf(stderr, "Service %s is already %sd. Skipping...\n", services[i], action);
> +                    break;
> +
> +                case 2:
> +                    fprintf(stderr, "\nUnable to %s service %s. (Service has no valid runlevel symlink).\n\n", action, services[i]);
> +                    break;
> +            }
> +
> +            // Break from for loop in case of a system error.
> +            if (r == -1) {
> +                r = 1;
> +                break;
> +            }
> +        }
> +
> +    } else if (strcmp(action, "boot-status") == 0) {
> +        // Print boot status for each service
> +        for(int i = 0; i < servicescnt; i++) {
> +            switch (get_boot_status(services[i])) {
> +                case -1:
> +                    r = 1;
> +                    fprintf(stderr, "\nSystem error occured. (Error: %m)\n\n");
> +                    break;
> +                
> +                case 0:
> +                    printf("%s is disabled on boot.\n", services[i]);
> +                    break;
> +                
> +                case 1:
> +                    printf("%s is enabled on boot.\n", services[i]);
> +                    break;
> +
> +                case 2:
> +                    printf("%s is not available for boot. (Service has no valid symlink in either %s or %s).\n", services[i], enabled_path, disabled_path);
> +                    break;
> +            }
> +            
> +            // Break from for loop in case of an error
> +            if (r == 1) {
> +                break;
> +            }
> +        }
> +    
> +    } else if (strcmp(action, "list-services") == 0) {
> +        // List all services for addon
> +        printf("\nServices for addon %s:\n", addon);
> +        for(int i = 0; i < servicescnt; i++) {
> +            printf("  %s\n", services[i]);
> +        }
> +        printf("\n");
> +
> +    } else {
> +        fprintf(stderr, "\nBad argument given.\n\n%s\n\n", usage);
> +        r = 1;
> +    }
> +
> +    // Cleanup
> +    for(int i = 0; i < servicescnt; i++) 
> +        free(services[i]);
> +    free(services);
> +
> +    return r;
> }
> -- 
> 2.37.3
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
> 

[PATCH v3 2/5] services.cgi: Fix status/actions on services with name != addon name

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 9983 bytes --]

* addonctrl's new functionality to control explicit addon services was
  implemented.
* Change 'Addon' column header to 'Addon Service' to be clear that
  it's not addons but services listed here.
* Services not matching the name of the addon now display the addon
  name between parentheses, so the user knows where the service comes
  from.
* When no valid runlevel symlink is found by addonctrl for a service,
  the 'enable on boot' checkbox is replaced by a small exclamation point
  with alt-text "No valid runlevel symlink was found for the initscript of
  this service." to inform user why a service can't be enabled.
* Added German and Dutch translation for above message.

Fixes: Bug#12935
Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
---
 html/cgi-bin/services.cgi | 114 ++++++++++++++++----------------------
 langs/de/cgi-bin/de.pl    |   1 +
 langs/en/cgi-bin/en.pl    |   1 +
 langs/nl/cgi-bin/nl.pl    |   1 +
 4 files changed, 51 insertions(+), 66 deletions(-)

diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
index 29926ecc3..de946d755 100644
--- a/html/cgi-bin/services.cgi
+++ b/html/cgi-bin/services.cgi
@@ -20,7 +20,7 @@
 ###############################################################################
 
 use strict;
-
+use feature "switch";
 # enable only the following on debugging purpose
 #use warnings;
 #use CGI::Carp 'fatalsToBrowser';
@@ -141,15 +141,22 @@ END
 	&Header::openbox('100%', 'left', "Addon - $Lang::tr{services}");
 	my $paramstr=$ENV{QUERY_STRING};
 	my @param=split(/!/, $paramstr);
-	if ($param[1] ne ''){
-		&General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]");
+	# Make sure action parameter is actually one of the allowed service actions
+	given ($param[1]) {
+		when ( ['start', 'stop', 'enable', 'disable'] ) {
+			# Make sure pak-name and service name don't contain any illegal character
+			if ( $param[0] !~ /[^a-zA-Z_0-9\-]/ &&
+			     $param[2] !~ /[^a-zA-Z_0-9\-]/ ) {
+				&General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]", "$param[2]");
+			}
+		}
 	}
 
 	print <<END
 <div align='center'>
 <table width='80%' cellspacing='1' class='tbl'>
 <tr>
-	<th align='center'><b>Addon</b></th>
+	<th align='left'><b>Addon $Lang::tr{service}</b></th>
 	<th align='center'><b>Boot</b></th>
 	<th align='center' colspan=2><b>$Lang::tr{'action'}</b></th>
 	<th align='center'><b>$Lang::tr{'status'}</b></th>
@@ -170,33 +177,35 @@ END
 	foreach my $pak (keys %paklist) {
 		my %metadata = &Pakfire::getmetadata($pak, "installed");
 			
+		my $service;
+
 		if ("$metadata{'Services'}") {
-			foreach my $service (split(/ /, "$metadata{'Services'}")) {
-				push(@addon_services, $service);
-			}
-		}
-	}
+			foreach $service (split(/ /, "$metadata{'Services'}")) {
+				$lines++;
+				if ($lines % 2) {
+					print "<tr>";
+					$col="bgcolor='$color{'color22'}'";
+				} else {
+					print "<tr>";
+					$col="bgcolor='$color{'color20'}'";
+				}
 
-	foreach (@addon_services) {
-		$lines++;
-		if ($lines % 2){
-			print "<tr>";
-			$col="bgcolor='$color{'color22'}'";
-		}else{
-			print "<tr>";
-			$col="bgcolor='$color{'color20'}'";
+				# Add addon name to displayname of service if servicename differs from addon
+				my $displayname = ($pak ne $service) ? "$service ($pak)" : $service;
+				print "<td align='left' $col width='31%'>$displayname</td> ";
+
+				my $status = isautorun($pak,$service,$col);
+				print "$status ";
+				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
+				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
+				my $status = isrunningaddon($pak,$service,$col);
+				$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
+
+				chomp($status);
+				print "$status";
+				print "</tr>";
+			}
 		}
-		print "<td align='left' $col width='31%'>$_</td> ";
-		my $status = isautorun($_,$col);
-		print "$status ";
-		print "<td align='center' $col width='8%'><a href='services.cgi?$_!start'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
-		print "<td align='center' $col width='8%'><a href='services.cgi?$_!stop'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
-		my $status = isrunningaddon($_,$col);
-		$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
-
-		chomp($status);
-		print "$status";
-		print "</tr>";
 	}
 
 	print "</table></div>\n";
@@ -215,51 +224,24 @@ END
 }
 
 sub isautorun (@) {
-	my ($cmd, $col) = @_;
-
-	# Init directory.
-	my $initdir = "/etc/rc.d/rc3.d/";
-
-	my $status = "<td align='center' $col></td>";
+	my ($pak, $service, $col) = @_;
+	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "boot-status", "$service");
+	my $testcmd = @testcmd[0];
+	my $status = "<td align='center' $col><img alt='$Lang::tr{'service boot setting unavailable'}' title='$Lang::tr{'service boot setting unavailable'}' src='/images/dialog-warning.png' border='0' width='16' height='16' /></td>";
 
-	# Check if autorun for the given cmd is enabled.
-	if ( &find_init("$cmd", "$initdir") ) {
+	# Check if autorun for the given service is enabled.
+	if ( $testcmd =~ /enabled\ on\ boot/ ) {
 		# Adjust status.
-		$status = "<td align='center' $col><a href='services.cgi?$_!disable'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
-	} else {
+		$status = "<td align='center' $col><a href='services.cgi?$pak!disable!$service'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
+	} elsif ( $testcmd =~ /disabled\ on\ boot/ ) {
 		# Adjust status.
-		$status = "<td align='center' $col><a href='services.cgi?$_!enable'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
+		$status = "<td align='center' $col><a href='services.cgi?$pak!enable!$service'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
 	}
 
 	# Return the status.
 	return $status;
 }
 
-sub find_init (@) {
-	my ($cmd, $dir) = @_;
-
-	# Open given init directory.
-	opendir (INITDIR, "$dir") || die "Cannot opendir $dir: $!";
-
-	# Read-in init files from directory.
-	my @inits = readdir(INITDIR);
-
-	# Close directory handle.
-	closedir(INITDIR);
-
-	# Loop through the directory.
-	foreach my $init (@inits) {
-		# Check if the current processed file belongs to the given command.
-		if ($init =~ /S\d+\d+$cmd\z/) {
-			# Found, return "1" - True.
-			return "1";
-		}
-        }
-
-	# Nothing found, return nothing.
-	return;
-}
-
 sub isrunning (@) {
 	my ($cmd, $col) = @_;
 	my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
@@ -313,7 +295,7 @@ sub isrunning (@) {
 }
 
 sub isrunningaddon (@) {
-	my ($cmd, $col) = @_;
+	my ($pak, $service, $col) = @_;
 
 	my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
 	my $pid = '';
@@ -321,7 +303,7 @@ sub isrunningaddon (@) {
 	my $exename;
 	my @memory;
 
-	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$cmd", "status");
+	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "status", "$service");
 	my $testcmd = @testcmd[0];
 
 	if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\ not\ running/){
diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
index 798abcffc..db7d117b0 100644
--- a/langs/de/cgi-bin/de.pl
+++ b/langs/de/cgi-bin/de.pl
@@ -2251,6 +2251,7 @@
 'server string' => 'Server String',
 'service' => 'Dienst',
 'service added' => 'Benutzerdefinierter Netzwerkdienst wurde hinzugefügt',
+'service boot setting unavailable' => 'Für das Initscript dieses Dienstes wurde kein gültiger Runlevel-Symlink gefunden.',
 'service name' => 'Name des Dienstes:',
 'service removed' => 'Benutzerdefinierter Netzwerkdienst wurde entfernt',
 'service updated' => 'Benutzerdefinierter Netzwerkdienst wurde aktualisiert',
diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
index f770e7cd9..60dca5be4 100644
--- a/langs/en/cgi-bin/en.pl
+++ b/langs/en/cgi-bin/en.pl
@@ -2306,6 +2306,7 @@
 'server string' => 'Server String',
 'service' => 'Service',
 'service added' => 'Custom network service added',
+'service boot setting unavailable' => 'No valid runlevel symlink was found for the initscript of this service.',
 'service name' => 'Service name:',
 'service removed' => 'Custom network service removed',
 'service updated' => 'Custom network service updated',
diff --git a/langs/nl/cgi-bin/nl.pl b/langs/nl/cgi-bin/nl.pl
index 49dabec99..4fd6955cc 100644
--- a/langs/nl/cgi-bin/nl.pl
+++ b/langs/nl/cgi-bin/nl.pl
@@ -1899,6 +1899,7 @@
 'server string' => 'Server String',
 'service' => 'Dienst',
 'service added' => 'Aangepaste netwerkdienst toegevoegd',
+'service boot setting unavailable' => 'Er werd voor het initscript van deze service geen geldige runlevel symlink gevonden.',
 'service name' => 'Dienstennaam:',
 'service removed' => 'Aangepaste netwerkdienst verwijderd',
 'service updated' => 'Aangepaste netwerkdienst bijgewerkt',
-- 
2.37.3


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

Re: [PATCH v3 2/5] services.cgi: Fix status/actions on services with name != addon name

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 10534 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> * addonctrl's new functionality to control explicit addon services was
>  implemented.
> * Change 'Addon' column header to 'Addon Service' to be clear that
>  it's not addons but services listed here.
> * Services not matching the name of the addon now display the addon
>  name between parentheses, so the user knows where the service comes
>  from.
> * When no valid runlevel symlink is found by addonctrl for a service,
>  the 'enable on boot' checkbox is replaced by a small exclamation point
>  with alt-text "No valid runlevel symlink was found for the initscript of
>  this service." to inform user why a service can't be enabled.
> * Added German and Dutch translation for above message.
> 
> Fixes: Bug#12935
> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> ---
> html/cgi-bin/services.cgi | 114 ++++++++++++++++----------------------
> langs/de/cgi-bin/de.pl    |   1 +
> langs/en/cgi-bin/en.pl    |   1 +
> langs/nl/cgi-bin/nl.pl    |   1 +
> 4 files changed, 51 insertions(+), 66 deletions(-)
> 
> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
> index 29926ecc3..de946d755 100644
> --- a/html/cgi-bin/services.cgi
> +++ b/html/cgi-bin/services.cgi
> @@ -20,7 +20,7 @@
> ###############################################################################
> 
> use strict;
> -
> +use feature "switch";
> # enable only the following on debugging purpose
> #use warnings;
> #use CGI::Carp 'fatalsToBrowser';
> @@ -141,15 +141,22 @@ END
> 	&Header::openbox('100%', 'left', "Addon - $Lang::tr{services}");
> 	my $paramstr=$ENV{QUERY_STRING};
> 	my @param=split(/!/, $paramstr);
> -	if ($param[1] ne ''){
> -		&General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]");
> +	# Make sure action parameter is actually one of the allowed service actions
> +	given ($param[1]) {
> +		when ( ['start', 'stop', 'enable', 'disable'] ) {
> +			# Make sure pak-name and service name don't contain any illegal character
> +			if ( $param[0] !~ /[^a-zA-Z_0-9\-]/ &&
> +			     $param[2] !~ /[^a-zA-Z_0-9\-]/ ) {
> +				&General::system("/usr/local/bin/addonctrl", "$param[0]", "$param[1]", "$param[2]");
> +			}
> +		}
> 	}
> 
> 	print <<END
> <div align='center'>
> <table width='80%' cellspacing='1' class='tbl'>
> <tr>
> -	<th align='center'><b>Addon</b></th>
> +	<th align='left'><b>Addon $Lang::tr{service}</b></th>
> 	<th align='center'><b>Boot</b></th>
> 	<th align='center' colspan=2><b>$Lang::tr{'action'}</b></th>
> 	<th align='center'><b>$Lang::tr{'status'}</b></th>
> @@ -170,33 +177,35 @@ END
> 	foreach my $pak (keys %paklist) {
> 		my %metadata = &Pakfire::getmetadata($pak, "installed");
> 			
> +		my $service;
> +
> 		if ("$metadata{'Services'}") {
> -			foreach my $service (split(/ /, "$metadata{'Services'}")) {
> -				push(@addon_services, $service);
> -			}
> -		}
> -	}
> +			foreach $service (split(/ /, "$metadata{'Services'}")) {
> +				$lines++;
> +				if ($lines % 2) {
> +					print "<tr>";
> +					$col="bgcolor='$color{'color22'}'";
> +				} else {
> +					print "<tr>";
> +					$col="bgcolor='$color{'color20'}'";
> +				}
> 
> -	foreach (@addon_services) {
> -		$lines++;
> -		if ($lines % 2){
> -			print "<tr>";
> -			$col="bgcolor='$color{'color22'}'";
> -		}else{
> -			print "<tr>";
> -			$col="bgcolor='$color{'color20'}'";
> +				# Add addon name to displayname of service if servicename differs from addon
> +				my $displayname = ($pak ne $service) ? "$service ($pak)" : $service;
> +				print "<td align='left' $col width='31%'>$displayname</td> ";
> +
> +				my $status = isautorun($pak,$service,$col);
> +				print "$status ";
> +				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
> +				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
> +				my $status = isrunningaddon($pak,$service,$col);
> +				$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
> +
> +				chomp($status);
> +				print "$status";
> +				print "</tr>";
> +			}
> 		}
> -		print "<td align='left' $col width='31%'>$_</td> ";
> -		my $status = isautorun($_,$col);
> -		print "$status ";
> -		print "<td align='center' $col width='8%'><a href='services.cgi?$_!start'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
> -		print "<td align='center' $col width='8%'><a href='services.cgi?$_!stop'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
> -		my $status = isrunningaddon($_,$col);
> -		$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
> -
> -		chomp($status);
> -		print "$status";
> -		print "</tr>";
> 	}
> 
> 	print "</table></div>\n";
> @@ -215,51 +224,24 @@ END
> }
> 
> sub isautorun (@) {
> -	my ($cmd, $col) = @_;
> -
> -	# Init directory.
> -	my $initdir = "/etc/rc.d/rc3.d/";
> -
> -	my $status = "<td align='center' $col></td>";
> +	my ($pak, $service, $col) = @_;
> +	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "boot-status", "$service");
> +	my $testcmd = @testcmd[0];
> +	my $status = "<td align='center' $col><img alt='$Lang::tr{'service boot setting unavailable'}' title='$Lang::tr{'service boot setting unavailable'}' src='/images/dialog-warning.png' border='0' width='16' height='16' /></td>";
> 
> -	# Check if autorun for the given cmd is enabled.
> -	if ( &find_init("$cmd", "$initdir") ) {
> +	# Check if autorun for the given service is enabled.
> +	if ( $testcmd =~ /enabled\ on\ boot/ ) {
> 		# Adjust status.
> -		$status = "<td align='center' $col><a href='services.cgi?$_!disable'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
> -	} else {
> +		$status = "<td align='center' $col><a href='services.cgi?$pak!disable!$service'><img alt='$Lang::tr{'deactivate'}' title='$Lang::tr{'deactivate'}' src='/images/on.gif' border='0' width='16' height='16' /></a></td>";
> +	} elsif ( $testcmd =~ /disabled\ on\ boot/ ) {
> 		# Adjust status.
> -		$status = "<td align='center' $col><a href='services.cgi?$_!enable'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
> +		$status = "<td align='center' $col><a href='services.cgi?$pak!enable!$service'><img alt='$Lang::tr{'activate'}' title='$Lang::tr{'activate'}' src='/images/off.gif' border='0' width='16' height='16' /></a></td>";
> 	}
> 
> 	# Return the status.
> 	return $status;
> }
> 
> -sub find_init (@) {
> -	my ($cmd, $dir) = @_;
> -
> -	# Open given init directory.
> -	opendir (INITDIR, "$dir") || die "Cannot opendir $dir: $!";
> -
> -	# Read-in init files from directory.
> -	my @inits = readdir(INITDIR);
> -
> -	# Close directory handle.
> -	closedir(INITDIR);
> -
> -	# Loop through the directory.
> -	foreach my $init (@inits) {
> -		# Check if the current processed file belongs to the given command.
> -		if ($init =~ /S\d+\d+$cmd\z/) {
> -			# Found, return "1" - True.
> -			return "1";
> -		}
> -        }
> -
> -	# Nothing found, return nothing.
> -	return;
> -}
> -
> sub isrunning (@) {
> 	my ($cmd, $col) = @_;
> 	my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
> @@ -313,7 +295,7 @@ sub isrunning (@) {
> }
> 
> sub isrunningaddon (@) {
> -	my ($cmd, $col) = @_;
> +	my ($pak, $service, $col) = @_;
> 
> 	my $status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
> 	my $pid = '';
> @@ -321,7 +303,7 @@ sub isrunningaddon (@) {
> 	my $exename;
> 	my @memory;
> 
> -	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$cmd", "status");
> +	my @testcmd = &General::system_output("/usr/local/bin/addonctrl", "$pak", "status", "$service");
> 	my $testcmd = @testcmd[0];
> 
> 	if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\ not\ running/){
> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> index 798abcffc..db7d117b0 100644
> --- a/langs/de/cgi-bin/de.pl
> +++ b/langs/de/cgi-bin/de.pl
> @@ -2251,6 +2251,7 @@
> 'server string' => 'Server String',
> 'service' => 'Dienst',
> 'service added' => 'Benutzerdefinierter Netzwerkdienst wurde hinzugefügt',
> +'service boot setting unavailable' => 'Für das Initscript dieses Dienstes wurde kein gültiger Runlevel-Symlink gefunden.',
> 'service name' => 'Name des Dienstes:',
> 'service removed' => 'Benutzerdefinierter Netzwerkdienst wurde entfernt',
> 'service updated' => 'Benutzerdefinierter Netzwerkdienst wurde aktualisiert',
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index f770e7cd9..60dca5be4 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -2306,6 +2306,7 @@
> 'server string' => 'Server String',
> 'service' => 'Service',
> 'service added' => 'Custom network service added',
> +'service boot setting unavailable' => 'No valid runlevel symlink was found for the initscript of this service.',
> 'service name' => 'Service name:',
> 'service removed' => 'Custom network service removed',
> 'service updated' => 'Custom network service updated',
> diff --git a/langs/nl/cgi-bin/nl.pl b/langs/nl/cgi-bin/nl.pl
> index 49dabec99..4fd6955cc 100644
> --- a/langs/nl/cgi-bin/nl.pl
> +++ b/langs/nl/cgi-bin/nl.pl
> @@ -1899,6 +1899,7 @@
> 'server string' => 'Server String',
> 'service' => 'Dienst',
> 'service added' => 'Aangepaste netwerkdienst toegevoegd',
> +'service boot setting unavailable' => 'Er werd voor het initscript van deze service geen geldige runlevel symlink gevonden.',
> 'service name' => 'Dienstennaam:',
> 'service removed' => 'Aangepaste netwerkdienst verwijderd',
> 'service updated' => 'Aangepaste netwerkdienst bijgewerkt',
> -- 
> 2.37.3
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
> 

[PATCH v3 3/5] services.cgi: minor cosmetics

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

* Singular 'Service' instead of plural 'Services' as column header of
  services table
* Sort list of services

Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
---
 html/cgi-bin/services.cgi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
index de946d755..e35b04cae 100644
--- a/html/cgi-bin/services.cgi
+++ b/html/cgi-bin/services.cgi
@@ -105,7 +105,7 @@ if ( $querry[0] =~ "processescpu"){
 <div align='center'>
 <table width='80%' cellspacing='1' class='tbl'>
 <tr>
-	<th align='left'><b>$Lang::tr{'services'}</b></th>
+	<th align='left'><b>$Lang::tr{'service'}</b></th>
 	<th align='center' ><b>$Lang::tr{'status'}</b></th>
 	<th align='center'><b>PID</b></th>
 	<th align='center'><b>$Lang::tr{'memory'}</b></th>
@@ -174,7 +174,7 @@ END
 	# Generate list of installed addon pak services
 	my %paklist = &Pakfire::dblist("installed");
 
-	foreach my $pak (keys %paklist) {
+	foreach my $pak (sort keys %paklist) {
 		my %metadata = &Pakfire::getmetadata($pak, "installed");
 			
 		my $service;
-- 
2.37.3


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

Re: [PATCH v3 3/5] services.cgi: minor cosmetics

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> * Singular 'Service' instead of plural 'Services' as column header of
>  services table
> * Sort list of services
> 
> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> ---
> html/cgi-bin/services.cgi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
> index de946d755..e35b04cae 100644
> --- a/html/cgi-bin/services.cgi
> +++ b/html/cgi-bin/services.cgi
> @@ -105,7 +105,7 @@ if ( $querry[0] =~ "processescpu"){
> <div align='center'>
> <table width='80%' cellspacing='1' class='tbl'>
> <tr>
> -	<th align='left'><b>$Lang::tr{'services'}</b></th>
> +	<th align='left'><b>$Lang::tr{'service'}</b></th>
> 	<th align='center' ><b>$Lang::tr{'status'}</b></th>
> 	<th align='center'><b>PID</b></th>
> 	<th align='center'><b>$Lang::tr{'memory'}</b></th>
> @@ -174,7 +174,7 @@ END
> 	# Generate list of installed addon pak services
> 	my %paklist = &Pakfire::dblist("installed");
> 
> -	foreach my $pak (keys %paklist) {
> +	foreach my $pak (sort keys %paklist) {
> 		my %metadata = &Pakfire::getmetadata($pak, "installed");
> 			
> 		my $service;
> -- 
> 2.37.3
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
> 

[PATCH v3 4/5] services.cgi: add restart action and restrict action usage

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 3255 bytes --]

* Add restart action to services.
* Only display available actions for a service:
  Start when service is stopped or Stop and Restart when a service
  is running.

Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
---
 html/cgi-bin/services.cgi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
index e35b04cae..4b379251e 100644
--- a/html/cgi-bin/services.cgi
+++ b/html/cgi-bin/services.cgi
@@ -143,7 +143,7 @@ END
 	my @param=split(/!/, $paramstr);
 	# Make sure action parameter is actually one of the allowed service actions
 	given ($param[1]) {
-		when ( ['start', 'stop', 'enable', 'disable'] ) {
+		when ( ['start', 'stop', 'restart', 'enable', 'disable'] ) {
 			# Make sure pak-name and service name don't contain any illegal character
 			if ( $param[0] !~ /[^a-zA-Z_0-9\-]/ &&
 			     $param[2] !~ /[^a-zA-Z_0-9\-]/ ) {
@@ -196,8 +196,6 @@ END
 
 				my $status = isautorun($pak,$service,$col);
 				print "$status ";
-				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
-				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
 				my $status = isrunningaddon($pak,$service,$col);
 				$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
 
@@ -307,7 +305,9 @@ sub isrunningaddon (@) {
 	my $testcmd = @testcmd[0];
 
 	if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\ not\ running/){
-		$status = "<td align='center' bgcolor='${Header::colourgreen}'><font color='white'><b>$Lang::tr{'running'}</b></font></td>";
+		$status = "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
+		$status .= "<td align='center' $col width='8%'><a href='services.cgi?$pak!restart!$service'><img alt='$Lang::tr{'restart'}' title='$Lang::tr{'restart'}' src='/images/reload.gif' border='0' /></a></td> ";
+		$status .= "<td align='center' bgcolor='${Header::colourgreen}'><font color='white'><b>$Lang::tr{'running'}</b></font></td>";
 		$testcmd =~ s/.* //gi;
 		$testcmd =~ s/[a-z_]//gi;
 		$testcmd =~ s/\[[0-1]\;[0-9]+//gi;
@@ -330,7 +330,8 @@ sub isrunningaddon (@) {
 		}
 		$status .="<td align='center' $col>$memory KB</td>";
 	}else{
-		$status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
+		$status = "<td align='center' $col width='16%' colspan=2><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
+		$status .= "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
 	}
 	return $status;
 }
-- 
2.37.3


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

Re: [PATCH v3 4/5] services.cgi: add restart action and restrict action usage

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> * Add restart action to services.
> * Only display available actions for a service:
>  Start when service is stopped or Stop and Restart when a service
>  is running.
> 
> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> ---
> html/cgi-bin/services.cgi | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
> index e35b04cae..4b379251e 100644
> --- a/html/cgi-bin/services.cgi
> +++ b/html/cgi-bin/services.cgi
> @@ -143,7 +143,7 @@ END
> 	my @param=split(/!/, $paramstr);
> 	# Make sure action parameter is actually one of the allowed service actions
> 	given ($param[1]) {
> -		when ( ['start', 'stop', 'enable', 'disable'] ) {
> +		when ( ['start', 'stop', 'restart', 'enable', 'disable'] ) {
> 			# Make sure pak-name and service name don't contain any illegal character
> 			if ( $param[0] !~ /[^a-zA-Z_0-9\-]/ &&
> 			     $param[2] !~ /[^a-zA-Z_0-9\-]/ ) {
> @@ -196,8 +196,6 @@ END
> 
> 				my $status = isautorun($pak,$service,$col);
> 				print "$status ";
> -				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
> -				print "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
> 				my $status = isrunningaddon($pak,$service,$col);
> 				$status =~ s/\^[\[[0-1]\;[0-9]+m//g;
> 
> @@ -307,7 +305,9 @@ sub isrunningaddon (@) {
> 	my $testcmd = @testcmd[0];
> 
> 	if ( $testcmd =~ /is\ running/ && $testcmd !~ /is\ not\ running/){
> -		$status = "<td align='center' bgcolor='${Header::colourgreen}'><font color='white'><b>$Lang::tr{'running'}</b></font></td>";
> +		$status = "<td align='center' $col width='8%'><a href='services.cgi?$pak!stop!$service'><img alt='$Lang::tr{'stop'}' title='$Lang::tr{'stop'}' src='/images/go-down.png' border='0' /></a></td> ";
> +		$status .= "<td align='center' $col width='8%'><a href='services.cgi?$pak!restart!$service'><img alt='$Lang::tr{'restart'}' title='$Lang::tr{'restart'}' src='/images/reload.gif' border='0' /></a></td> ";
> +		$status .= "<td align='center' bgcolor='${Header::colourgreen}'><font color='white'><b>$Lang::tr{'running'}</b></font></td>";
> 		$testcmd =~ s/.* //gi;
> 		$testcmd =~ s/[a-z_]//gi;
> 		$testcmd =~ s/\[[0-1]\;[0-9]+//gi;
> @@ -330,7 +330,8 @@ sub isrunningaddon (@) {
> 		}
> 		$status .="<td align='center' $col>$memory KB</td>";
> 	}else{
> -		$status = "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
> +		$status = "<td align='center' $col width='16%' colspan=2><a href='services.cgi?$pak!start!$service'><img alt='$Lang::tr{'start'}' title='$Lang::tr{'start'}' src='/images/go-up.png' border='0' /></a></td>";
> +		$status .= "<td align='center' bgcolor='${Header::colourred}'><font color='white'><b>$Lang::tr{'stopped'}</b></font></td><td colspan='2' $col></td>";
> 	}
> 	return $status;
> }
> -- 
> 2.37.3
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
> 

[PATCH v3 5/5] services.cgi: add link to addon config if ui exists for it

[Robin Roevens]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

* If a cgi file exists with the same name as an addon, the
  displayed service will be a link to that cgi file.

Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
---
 html/cgi-bin/services.cgi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
index 4b379251e..14ed01c50 100644
--- a/html/cgi-bin/services.cgi
+++ b/html/cgi-bin/services.cgi
@@ -192,6 +192,10 @@ END
 
 				# Add addon name to displayname of service if servicename differs from addon
 				my $displayname = ($pak ne $service) ? "$service ($pak)" : $service;
+				if ( -e "/srv/web/ipfire/cgi-bin/$pak.cgi" ) {
+					$displayname = ($pak ne $service) ? "$service (<a href=\'$pak.cgi\'>$pak</a>)" : "<a href=\'$pak.cgi\'>$service</a>";
+				} 
+
 				print "<td align='left' $col width='31%'>$displayname</td> ";
 
 				my $status = isautorun($pak,$service,$col);
-- 
2.37.3


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.

Re: [PATCH v3 5/5] services.cgi: add link to addon config if ui exists for it

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> * If a cgi file exists with the same name as an addon, the
>  displayed service will be a link to that cgi file.
> 
> Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> ---
> html/cgi-bin/services.cgi | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi
> index 4b379251e..14ed01c50 100644
> --- a/html/cgi-bin/services.cgi
> +++ b/html/cgi-bin/services.cgi
> @@ -192,6 +192,10 @@ END
> 
> 				# Add addon name to displayname of service if servicename differs from addon
> 				my $displayname = ($pak ne $service) ? "$service ($pak)" : $service;
> +				if ( -e "/srv/web/ipfire/cgi-bin/$pak.cgi" ) {
> +					$displayname = ($pak ne $service) ? "$service (<a href=\'$pak.cgi\'>$pak</a>)" : "<a href=\'$pak.cgi\'>$service</a>";
> +				} 
> +
> 				print "<td align='left' $col width='31%'>$displayname</td> ";
> 
> 				my $status = isautorun($pak,$service,$col);
> -- 
> 2.37.3
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
> 

Re: [PATCH v3 0/5] Fix Bug#12935 + cosmetic changes/enhancements

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 2863 bytes --]

Hello Robin,

> On 11 Oct 2022, at 23:01, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> Hi all
> 
> After carefully reviewing and adopting all comments Michael had about
> the code in previous patchset, here is again a new version of it.
> 
> Only PATCH 1 was changed since v2:
> - propper initialization of variables where required preventing possible
>  undefined behaviour
> - more reliable error checking: all functions now return a meaningfull
>  integer one way or another to indicate if and what kind of error
>  happend. No need anymore to set errno to 0 manually as it no longer
>  depended on.
> - No more checking against NULL making many comparisions easier for the
>  eye
> - fix some possible memory leaks
> - fix a possible unallocation of not yet allocated memory
> - in case of a system error, a descriptive error is shown instead of
>  the number, using the %m directive.
> 
> Hoping to pass the required strict evaluation this time :-).

Yes, I think so.

It is a huge patchset, so hard to review, but I am happy with it and happy to put my stamp on it :)

Thank you very much for your very hard work on this and all those hours that you put in.

What is coming next? :)

-Michael

> For reference, the content of the summary mail that was sent with v1 of
> the patch:
> ---
> This patchset fixes Bug#12935
> (https://bugzilla.ipfire.org/show_bug.cgi?id=12935)
> 
> Summary:
> Addons where the initscript does not match the addon-name and addons with
> multiple initscripts are now listed on services.cgi since CU170.
> But addonctrl still expected addon name to be equal to
> initscript name; Hence starting/stopping/enabling/disabling of such
> addons was not possible.
> This has always been like that, but that problem was hidden as
> services.cgi also did not display those addon services.
> 
> After discussing this with Adolf on the Bug report, we concluded that we
> should adapt addonctrl to work with the new addon metadata
> Services-field instead.
> 
> I basically rewrote addonctrl to not only use the new services metadata
> but also to have better errorchecking and added the posibility to check
> if a service is currently enabled or disabled.
> As a result services.cgi no longer has to go checking the precense of
> runlevel initscripts, but can just ask addonctrl.
> I also added a warning to services.cgi if a runlevel initscript does not
> exists, to prevent the user from wondering why he can't enable a
> specific service. (Adolf pointed out some services don't install
> runlevel initscripts by default)
> 
> More details in the bugreport and in the commit-messages of the patches.
> 
> Regards
> Robin
> 
> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.
>