Tidy C code based on review feedback

c/relexec.c

@@ -3,79 +3,79 @@
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
-#include <sysexits.h>
 #include <libgen.h>
-#include <linux/limits.h>
+#include <limits.h>
+#include <err.h>
 
 
-ssize_t joinpath(char *root, const char *rel, size_t rootsize) {
+ssize_t joinpath(char *dst, size_t dstsize, const char *rel) {
     size_t rel_len = strlen(rel);
 
     /* If rel is absolute, replace root */
     if (rel[0] == '/') {
-        if (rel_len + 1 > rootsize) {
+        if (rel_len + 1 > dstsize) {
             return -1;
         }
-        strcpy(root, rel);
+        strcpy(dst, rel);
         return rel_len;
     }
 
     // dirname may modify the argument or return static memory, so use a copy
-    char *buf = strdup(root);
-    char *rootdir = dirname(buf);
+    char pathbuf[PATH_MAX];
+    strcpy(pathbuf, dst);
+    char *rootdir = dirname(pathbuf);
 
-    ssize_t ret = -1;
+    // copy rootdir into and ensure a trailing slash
+    // also check that there is space to append rel to dst, return -1
+    // if insufficient, with errno set.
     size_t end = strlen(rootdir);
-    if (root[end - 1] != '/') {
-        if (end + rel_len + 2 > rootsize) {
-            goto end;
+    if (dst[end - 1] != '/') {
+        if (end + rel_len + 2 > dstsize) {
+            errno = EOVERFLOW;
+            return -1;
         }
-        strcpy(root, rootdir);
-        root[end++] = '/';
+        strcpy(dst, rootdir);
+        dst[end++] = '/';
     } else {
-        if (end + rel_len + 1 > rootsize) {
-            goto end;
+        if (end + rel_len + 1 > dstsize) {
+            errno = EOVERFLOW;
+            return -1;
         }
-        strcpy(root, rootdir);
+        strcpy(dst, rootdir);
     }
 
-    strcpy(root + end, rel);
-    ret = end + rel_len;
-
-    end:
-        free(buf);
-        return ret;
+    strcpy(dst + end, rel);
+    return end + rel_len;
 }
 
-static char _linktarget[PATH_MAX];
+static char linktarget[PATH_MAX];
 
 
 /* Follow symlinks until we find a real file or another error occurs.
  *
  * Return a pointer to static char data on success, or NULL on error, in
  * which case errno will be set.
  */
-const char *follow_link(const char *link) {
+static const char *follow_link(const char *link) {
     ssize_t res_len;
     char linkbuf[PATH_MAX];
-    strcpy(_linktarget, link);
+    strcpy(linktarget, link);
 
     for (;;) {
-        res_len = readlink(_linktarget, linkbuf, PATH_MAX);
+        res_len = readlink(linktarget, linkbuf, PATH_MAX);
         if (res_len != -1) {
             linkbuf[res_len] = '\0';
 
             if (linkbuf[0] == '/') {
-                strcpy(_linktarget, linkbuf);
+                strcpy(linktarget, linkbuf);
             } else {
-                if (joinpath(_linktarget, linkbuf, PATH_MAX) < 0) {
-                    errno = EOVERFLOW;
+                if (joinpath(linktarget, PATH_MAX, linkbuf) < 0) {
                     return NULL;
                 }
             }
         } else if (errno == EINVAL) {
             // Was not a link, therefore we're done
-            return _linktarget;
+            return linktarget;
         } else {
             // Another error
             return NULL;
@@ -85,47 +85,45 @@ const char *follow_link(const char *link) {
 
 
 int main(int argc, const char **argv) {
-    char *newargv[ARG_MAX];
+    char **newargv;
     char newinterp[PATH_MAX];
     size_t bs;
 
     if (argc < 3) {
-        fprintf(stderr, "usage: relexec <interpreter> <script>\n");
+        strcpy(newinterp, argv[0]);
+        char *base = basename(newinterp);
+        fprintf(stderr, "usage: %s <interpreter> <script>\n", base);
         return 2;
     }
 
     if (strlen(argv[2]) > PATH_MAX) {
-        fprintf(stderr, "relexec: path too long\n");
-        return 2;
+        err(2, "path too long\n");
     }
 
     /* Assemble the path to run */
     const char *linkdest = follow_link(argv[2]);
     if (!linkdest) {
-        fprintf(
-            stderr,
-            "Error reading %s: %s\n",
-            argv[2], strerror(errno)
-        );
-        return 2;
+        err(2, "error reading %s", argv[2]);
     }
 
     strcpy(newinterp, linkdest);
 
-    if (joinpath(newinterp, argv[1], PATH_MAX) < 0) {
-        fprintf(stderr, "relexec: path too long\n");
-        return 2;
+    if (joinpath(newinterp, PATH_MAX, argv[1]) < 0) {
+        err(2, "path too long\n");
     }
 
     /* Build argv, dropping the interpreter argument */
+    newargv = malloc(argc * sizeof(char *));
+    if (NULL == newargv) {
+        err(1, "Failed to allocate memory");
+    }
     newargv[0] = newinterp;
-    memmove(newargv + 1, argv + 2, sizeof(char **) * (argc - 2));
+    memcpy(newargv + 1, argv + 2, sizeof(char *) * (argc - 2));
     newargv[argc - 1] = NULL;
 
     /* Exec the target */
     execv(newinterp, newargv);
 
     /* If exec failed, return an error */
-    fprintf(stderr, "%s: %s\n", newinterp, strerror(errno));
-    return 127;
+    err(127, "failed to execute %s", newinterp);
 }