refactor: centralize and improve error handling and path construction

Makefile

@@ -1,3 +1,4 @@
+
 CC = gcc
 CFLAGS = -Wall -Wextra -std=c99 -Iinclude
 PREFIX ?= /usr
@@ -14,9 +15,9 @@ SOUND_TARGET = keyvibe-audio
 KEYBOARD_TARGET = keyvibe-input
 
 # Sources
-KeyVibe_SOURCE = src/keyvibe_main.c
-SOUND_SOURCE = src/keyvibe_audio.c
-KEYBOARD_SOURCE = src/keyvibe_input.c
+KeyVibe_SOURCE = src/keyvibe_main.c src/utils.c
+SOUND_SOURCE = src/keyvibe_audio.c src/utils.c
+KEYBOARD_SOURCE = src/keyvibe_input.c src/utils.c
 
 # Install paths
 BINDIR = $(PREFIX)/bin
@@ -28,13 +29,13 @@ all: $(KeyVibe_TARGET) $(SOUND_TARGET) $(KEYBOARD_TARGET)
 
 # Build main launcher (needs json-c)
 $(KeyVibe_TARGET): $(KeyVibe_SOURCE)
-	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $< -ljson-c -lpthread
+	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $^ -ljson-c -lpthread
 
 $(SOUND_TARGET): $(SOUND_SOURCE)
-	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $< $(LDFLAGS_SOUND)
+	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $^ $(LDFLAGS_SOUND)
 
 $(KEYBOARD_TARGET): $(KEYBOARD_SOURCE)
-	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $< $(LDFLAGS_KEYBOARD)
+	$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ $^ $(LDFLAGS_KEYBOARD)
 
 clean:
 	rm -f $(KeyVibe_TARGET) $(SOUND_TARGET) $(KEYBOARD_TARGET)

include/utils.h

@@ -0,0 +1,22 @@
+
+#ifndef KEYVIBE_UTILS_H
+#define KEYVIBE_UTILS_H
+
+#include <stddef.h>
+#include <sys/types.h>
+
+// Returns 1 on success, 0 on truncation/error
+int safe_snprintf(char *dst, size_t dst_sz, const char *fmt, ...);
+
+// fprintf to stderr with printf-style formatting. Returns 1 for convenient
+// `return errorf(...)` usage.
+int errorf(const char *fmt, ...);
+
+// Common runtime helpers
+const char *get_runtime_dir(void);
+void build_pidfile_path(char *buffer, size_t buflen);
+int read_pidfile(const char *path, pid_t *out_pid);
+int write_pidfile(const char *path, pid_t pid);
+int process_is_running(pid_t pid);
+
+#endif // KEYVIBE_UTILS_H

src/keyvibe_audio.c

@@ -1,4 +1,6 @@
+
 #define _GNU_SOURCE
+#include "utils.h"
 #include <errno.h>
 #include <json-c/json.h>
 #include <libgen.h>
@@ -58,7 +60,9 @@ static void get_full_path(char *buffer, size_t buffer_size,
     strncpy(buffer, filename, buffer_size - 1);
     buffer[buffer_size - 1] = '\0';
   } else {
-    snprintf(buffer, buffer_size, "%s/%s", base_dir, filename);
+    if (!safe_snprintf(buffer, buffer_size, "%s/%s", base_dir, filename)) {
+      buffer[0] = '\0';
+    }
   }
 }
 
@@ -256,9 +260,11 @@ int init_audio() {
     return -1;
   }
   sf_close(test_sf);
-  printf("Sound file info: %ld frames, %d channels, %d Hz\n",
-         g_sound_pack.sf_info.frames, g_sound_pack.sf_info.channels,
-         g_sound_pack.sf_info.samplerate);
+  if (g_verbose) {
+    printf("Sound file info: %ld frames, %d channels, %d Hz\n",
+           g_sound_pack.sf_info.frames, g_sound_pack.sf_info.channels,
+           g_sound_pack.sf_info.samplerate);
+  }
   return 0;
 }
 
@@ -483,7 +489,8 @@ int parse_keyboard_event(const char *json_line, int *key_code,
 }
 
 void cleanup() {
-  printf("Cleaning up...\n");
+  if (g_verbose)
+    printf("Cleaning up...\n");
   usleep(500000);
   for (int i = 0; i < 256; i++) {
     if (g_sound_pack.multi_key_mappings[i].press) {
@@ -508,8 +515,10 @@ static int read_mute_state() {
   if (!rd || strlen(rd) == 0) {
     rd = "/tmp";
   }
-  snprintf(mute_file, sizeof(mute_file), "%s/keyvibe-mute-%d", rd,
-           (int)getuid());
+  if (!safe_snprintf(mute_file, sizeof(mute_file), "%s/keyvibe-mute-%d", rd,
+                     (int)getuid())) {
+    return 0;
+  }
 
   FILE *f = fopen(mute_file, "r");
   if (!f) {
@@ -541,9 +550,11 @@ int main(int argc, char *argv[]) {
     if (volume_percent > 100)
       volume_percent = 100;
     g_volume = volume_percent / 100.0f;
-    printf("Volume set to: %d%%\n", volume_percent);
+    if (g_verbose)
+      printf("Volume set to: %d%%\n", volume_percent);
   } else {
-    printf("Volume set to: 50%% (default)\n");
+    if (g_verbose)
+      printf("Volume set to: 50%% (default)\n");
   }
   if (argc >= 4) {
     g_verbose = atoi(argv[3]);
@@ -594,7 +605,8 @@ int main(int argc, char *argv[]) {
     if (FD_ISSET(STDIN_FILENO, &readfds)) {
       if (fgets(line, sizeof(line), stdin) == NULL) {
         if (feof(stdin)) {
-          printf("EOF reached on stdin\n");
+          if (g_verbose)
+            printf("EOF reached on stdin\n");
         } else {
           perror("fgets");
         }

src/keyvibe_input.c

@@ -1,4 +1,5 @@
-// Renamed from get_key_presses.c to keyvibe_input.c (inlined)
+
+#include "utils.h"
 #include <errno.h>
 #include <stdbool.h>
 #include <stdint.h>
@@ -50,8 +51,7 @@ static int open_restricted(const char *path, int flags, void *user_data) {
   (void)user_data;
   int fd = open(path, flags);
   if (fd < 0)
-    fprintf(stderr, "Failed to open %s because of %s.\n", path,
-            strerror(errno));
+    errorf("Failed to open %s because of %s.\n", path, strerror(errno));
   return fd < 0 ? -errno : fd;
 }
 
@@ -128,11 +128,9 @@ static int run_mainloop(struct libinput *libinput) {
   fd.fd = libinput_get_fd(libinput);
   fd.events = POLLIN;
   fd.revents = 0;
-  if (handle_events(libinput) != 0) {
-    fprintf(stderr, "Expected device added events on startup but got none. "
-                    "Maybe you don't have the right permissions?\n");
-    return -1;
-  }
+  if (handle_events(libinput) != 0)
+    return errorf("Expected device added events on startup but got none. Maybe "
+                  "you don't have the right permissions?\n");
   while (poll(&fd, 1, -1) > -1)
     handle_events(libinput);
   return 0;
@@ -170,23 +168,19 @@ int main(int argc, char *argv[]) {
     case '?':
       break;
     default:
-      fprintf(stderr, "%s: Invalid option `-%c`.\n", argv[0], opt);
+      errorf("%s: Invalid option `-%c`.\n", argv[0], opt);
       break;
     }
   }
   struct udev *udev = udev_new();
-  if (udev == NULL) {
-    fprintf(stderr, "Failed to initialize udev.\n");
-    return UDEV_FAILED;
-  }
+  if (udev == NULL)
+    return errorf("Failed to initialize udev.\n");
   struct libinput *libinput =
       libinput_udev_create_context(&interface, NULL, udev);
-  if (!libinput) {
-    fprintf(stderr, "Failed to initialize libinput from udev.\n");
-    return LIBINPUT_FAILED;
-  }
+  if (!libinput)
+    return errorf("Failed to initialize libinput from udev.\n");
   if (libinput_udev_assign_seat(libinput, "seat0") != 0) {
-    fprintf(stderr, "Failed to set seat.\n");
+    errorf("Failed to set seat.\n");
     libinput_unref(libinput);
     udev_unref(udev);
     return SEAT_FAILED;

src/keyvibe_main.c

@@ -1,6 +1,7 @@
 #define _POSIX_C_SOURCE 200809L
 
 #include "config.h"
+#include "utils.h"
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -9,6 +10,7 @@
 #include <pthread.h>
 #include <pwd.h>
 #include <signal.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -135,9 +137,8 @@ static int resolve_sound_base_dir(const char *sound_name, char *out_basedir,
   if (user_audio_dir[0]) {
     char p[MAX_PATH_LENGTH];
     if ((size_t)snprintf(p, sizeof(p), "%s/%s/config.json", user_audio_dir,
-                         sound_name) >= sizeof(p)) {
-      // Path too long, skip user dir and try system dir
-    } else if (access(p, R_OK) == 0) {
+                         sound_name) < sizeof(p) &&
+        access(p, R_OK) == 0) {
       strncpy(out_basedir, user_audio_dir, out_sz - 1);
       out_basedir[out_sz - 1] = '\0';
       return 1;
@@ -208,17 +209,11 @@ static char *get_user_config_path(char *buffer, size_t buflen) {
   }
   if (!home)
     return NULL;
-  snprintf(buffer, buflen, "%s/%s", home, USER_CONFIG_FILENAME);
+  if (!safe_snprintf(buffer, buflen, "%s/%s", home, USER_CONFIG_FILENAME))
+    return NULL;
   return buffer;
 }
 
-static const char *get_runtime_dir() {
-  const char *rd = getenv("XDG_RUNTIME_DIR");
-  if (rd && strlen(rd) > 0)
-    return rd;
-  return "/tmp";
-}
-
 static int build_paths_for_sound(const char *sound_name, char *out_config_path,
                                  size_t out_config_sz, char *out_sound_dir,
                                  size_t out_sound_sz) {
@@ -238,48 +233,27 @@ static int build_paths_for_sound(const char *sound_name, char *out_config_path,
     fprintf(stderr, "Error: sound pack name too long for directory buffer.\n");
     return 0;
   }
-  snprintf(out_config_path, out_config_sz, "%s/%s%s", basedir, sound_name,
-           cfg_suffix);
-  snprintf(out_sound_dir, out_sound_sz, "%s/%s", basedir, sound_name);
+  if (!safe_snprintf(out_config_path, out_config_sz, "%s/%s%s", basedir,
+                     sound_name, cfg_suffix))
+    return 0;
+  if (!safe_snprintf(out_sound_dir, out_sound_sz, "%s/%s", basedir, sound_name))
+    return 0;
   return 1;
 }
 
-static void build_pidfile_path(char *buffer, size_t buflen) {
-  const char *rd = get_runtime_dir();
-  snprintf(buffer, buflen, "%s/keyvibe-%d.pid", rd, (int)getuid());
-}
-
-static int read_pidfile(const char *path, pid_t *out_pid) {
-  FILE *f = fopen(path, "r");
-  if (!f)
+// Helper: ensure daemon is running, return its pid or print a consistent error
+static int require_running_pid(pid_t *out_pid) {
+  build_pidfile_path(pidfile_path, sizeof(pidfile_path));
+  pid_t running_pid = 0;
+  if (!read_pidfile(pidfile_path, &running_pid) ||
+      !process_is_running(running_pid)) {
+    fprintf(stderr, "KeyVibe: not running.\n");
     return 0;
-  long val = -1;
-  if (fscanf(f, "%ld", &val) == 1 && val > 0) {
-    *out_pid = (pid_t)val;
-    fclose(f);
-    return 1;
   }
-  fclose(f);
-  return 0;
-}
-
-static int write_pidfile(const char *path, pid_t pid) {
-  FILE *f = fopen(path, "w");
-  if (!f)
-    return 0;
-  fprintf(f, "%ld\n", (long)pid);
-  fclose(f);
+  *out_pid = running_pid;
   return 1;
 }
 
-static int process_is_running(pid_t pid) {
-  if (pid <= 0)
-    return 0;
-  if (kill(pid, 0) == 0)
-    return 1;
-  return 0;
-}
-
 static void daemonize_self() {
   pid_t pid = fork();
   if (pid < 0)
@@ -558,11 +532,8 @@ int main(int argc, char *argv[]) {
       flag_stop = 1;
       break;
     case 'm': {
-      build_pidfile_path(pidfile_path, sizeof(pidfile_path));
       pid_t running_pid = 0;
-      if (!read_pidfile(pidfile_path, &running_pid) ||
-          !process_is_running(running_pid)) {
-        fprintf(stderr, "KeyVibe: not running.\n");
+      if (!require_running_pid(&running_pid)) {
         if (sound_name_owned) {
           free(sound_name);
         }
@@ -576,11 +547,8 @@ int main(int argc, char *argv[]) {
       return 0;
     }
     case 'u': {
-      build_pidfile_path(pidfile_path, sizeof(pidfile_path));
       pid_t running_pid = 0;
-      if (!read_pidfile(pidfile_path, &running_pid) ||
-          !process_is_running(running_pid)) {
-        fprintf(stderr, "KeyVibe: not running.\n");
+      if (!require_running_pid(&running_pid)) {
         if (sound_name_owned) {
           free(sound_name);
         }
@@ -609,44 +577,7 @@ int main(int argc, char *argv[]) {
     case 'l':
       list_sounds = 1;
       break;
-    case 1003: {
-      build_pidfile_path(pidfile_path, sizeof(pidfile_path));
-      pid_t running_pid = 0;
-      if (!read_pidfile(pidfile_path, &running_pid) ||
-          !process_is_running(running_pid)) {
-        fprintf(stderr, "KeyVibe: not running.\n");
-        if (sound_name_owned) {
-          free(sound_name);
-        }
-        return 1;
-      }
-      // Write mute state to file instead of using signals
-      write_mute_state(1);
-      printf("KeyVibe muted.\n");
-      if (sound_name_owned) {
-        free(sound_name);
-      }
-      return 0;
-    }
-    case 1004: {
-      build_pidfile_path(pidfile_path, sizeof(pidfile_path));
-      pid_t running_pid = 0;
-      if (!read_pidfile(pidfile_path, &running_pid) ||
-          !process_is_running(running_pid)) {
-        fprintf(stderr, "KeyVibe: not running.\n");
-        if (sound_name_owned) {
-          free(sound_name);
-        }
-        return 1;
-      }
-      // Write mute state to file instead of using signals
-      write_mute_state(0);
-      printf("KeyVibe unmuted.\n");
-      if (sound_name_owned) {
-        free(sound_name);
-      }
-      return 0;
-    }
+    /* removed legacy long mute/unmute cases; use -m/-u or --mute/--unmute */
     case 'h':
       print_usage(argv[0]);
       if (sound_name_owned) {
@@ -674,21 +605,10 @@ int main(int argc, char *argv[]) {
   build_pidfile_path(pidfile_path, sizeof(pidfile_path));
   if (flag_stop) {
     pid_t running_pid = 0;
-    if (!read_pidfile(pidfile_path, &running_pid)) {
-      fprintf(stderr, "KeyVibe: no pidfile found at %s\n", pidfile_path);
+    if (!require_running_pid(&running_pid))
       return 1;
-    }
-    if (!process_is_running(running_pid)) {
-      fprintf(stderr,
-              "KeyVibe: process %ld not running, removing stale pidfile\n",
-              (long)running_pid);
-      unlink(pidfile_path);
-      return 1;
-    }
-    if (kill(running_pid, SIGTERM) != 0) {
-      perror("kill");
-      return 1;
-    }
+    if (kill(running_pid, SIGTERM) != 0)
+      return (perror("kill"), 1);
     for (int i = 0; i < 30; i++) {
       if (!process_is_running(running_pid)) {
         unlink(pidfile_path);
@@ -700,8 +620,7 @@ int main(int argc, char *argv[]) {
       ts.tv_nsec = 100000000L;
       nanosleep(&ts, NULL);
     }
-    fprintf(stderr, "KeyVibe: process did not stop in time\n");
-    return 1;
+    return errorf("KeyVibe: process did not stop in time\n");
   }
   if (override_config) {
     if (cli_sound != NULL)
@@ -730,14 +649,10 @@ int main(int argc, char *argv[]) {
            "%s/keyvibe-input", KeyVibe_BIN_DIR);
   snprintf(sound_player_path, sizeof(sound_player_path), "%s/keyvibe-audio",
            KeyVibe_BIN_DIR);
-  if (access(get_key_presses_path, X_OK) != 0) {
-    fprintf(stderr, "Error: Cannot find or execute %s\n", get_key_presses_path);
-    return 1;
-  }
-  if (access(sound_player_path, X_OK) != 0) {
-    fprintf(stderr, "Error: Cannot find or execute %s\n", sound_player_path);
-    return 1;
-  }
+  if (access(get_key_presses_path, X_OK) != 0 ||
+      access(sound_player_path, X_OK) != 0)
+    return errorf("Error: Cannot find or execute required binaries in %s\n",
+                  KeyVibe_BIN_DIR);
   signal(SIGINT, cleanup_processes);
   signal(SIGTERM, cleanup_processes);
   if (flag_daemon) {

src/utils.c

@@ -0,0 +1,69 @@
+#define _POSIX_C_SOURCE 200809L
+#include "utils.h"
+#include "config.h"
+#include <pwd.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+int safe_snprintf(char *dst, size_t dst_sz, const char *fmt, ...) {
+  if (!dst || dst_sz == 0)
+    return 0;
+  va_list ap;
+  va_start(ap, fmt);
+  int n = vsnprintf(dst, dst_sz, fmt, ap);
+  va_end(ap);
+  return (n >= 0 && (size_t)n < dst_sz);
+}
+
+int errorf(const char *fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  vfprintf(stderr, fmt, ap);
+  va_end(ap);
+  return 1;
+}
+
+const char *get_runtime_dir(void) {
+  const char *rd = getenv("XDG_RUNTIME_DIR");
+  if (rd && strlen(rd) > 0)
+    return rd;
+  return "/tmp";
+}
+
+void build_pidfile_path(char *buffer, size_t buflen) {
+  const char *rd = get_runtime_dir();
+  safe_snprintf(buffer, buflen, "%s/keyvibe-%d.pid", rd, (int)getuid());
+}
+
+int read_pidfile(const char *path, pid_t *out_pid) {
+  FILE *f = fopen(path, "r");
+  if (!f)
+    return 0;
+  long val = -1;
+  if (fscanf(f, "%ld", &val) == 1 && val > 0) {
+    *out_pid = (pid_t)val;
+    fclose(f);
+    return 1;
+  }
+  fclose(f);
+  return 0;
+}
+
+int write_pidfile(const char *path, pid_t pid) {
+  FILE *f = fopen(path, "w");
+  if (!f)
+    return 0;
+  fprintf(f, "%ld\n", (long)pid);
+  fclose(f);
+  return 1;
+}
+
+int process_is_running(pid_t pid) {
+  if (pid <= 0)
+    return 0;
+  return kill(pid, 0) == 0;
+}