Code readability improvements

src/fuse_operations.c

@@ -48,6 +48,12 @@
 
 int auto_create_perm = GRANT_TEMP;
 
+/*
+ * Sets the default permission granted by file creation.
+ *
+ * @param val: the default permission to grant. Should be one of GRANT_TEMP,
+ * GRANT_TEM and 0. When set to 0, grants no permissions.
+ */
 void set_auto_create_perm(int val) { auto_create_perm = val; }
 
 static void *xmp_init(struct fuse_conn_info *conn, struct fuse_config *cfg) {
@@ -397,8 +403,8 @@ static int xmp_chmod(const char *path, mode_t mode, struct fuse_file_info *fi) {
 }
 
 /**
- * This filesystem is not designed for multiuser operation (e.g. with
+ * This filesystem is not designed for multiuser operation (with
- * allow_other) so there is little point in having chown implemnted
+ * allow_other option) so there is little point in having chown implemnted
  */
 static int xmp_chown(const char *path, uid_t uid, gid_t gid,
                      struct fuse_file_info *fi) {
@@ -466,8 +472,6 @@ static int xmp_create(const char *path, mode_t mode,
 
   pi = get_process_info(fc->pid);
 
-  // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
-
   if (auto_create_perm != 0) {
     if (!interactive_access(path, pi, auto_create_perm)) {
       free(pi.name);

src/fuse_operations.h

@@ -14,6 +14,13 @@
 #include <fuse3/fuse.h>
 
 const struct fuse_operations *get_fuse_operations();
+
+/*
+ * Sets the default permission granted by file creation.
+ *
+ * @param val: the default permission to grant. Should be one of GRANT_TEMP,
+ * GRANT_TEM and 0. When set to 0, grants no permissions.
+ */
 void set_auto_create_perm(int val);
 
 #endif

src/perm_permissions_table.c

@@ -23,45 +23,27 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+// Global pointer to the SQLite database storing permanent permissions
 sqlite3 *perm_database = NULL;
 const char *const table_name = "permissions";
-// one row corresponds to a permission to access one file for one executable
+// Each row represents a permission to access a specific file for an executable
 const int column_count = 3;
+// Column names in the permissions table
 const char *const schema[] = {"executable", "filename", "mode"};
+// Expected SQL data types for each column
 const char *const types[] = {"TEXT", "TEXT", "INTEGER"};
 uid_t ruid, euid, current_uid;
-pthread_mutex_t uid_switch = PTHREAD_MUTEX_INITIALIZER;
-
-void set_db_fsuid() {
-  pthread_mutex_lock(&uid_switch);
-  if (current_uid == ruid)
-    return;
-
-  int status = -1;
-
-  status = setfsuid(ruid);
-  if (status < 0) {
-    fprintf(stderr, "[ICFS] Couldn't set uid to %d.\n", ruid);
-    exit(status);
-  }
-  pthread_mutex_unlock(&uid_switch);
-}
-
-void set_real_fsuid() {
-  pthread_mutex_lock(&uid_switch);
-  if (current_uid == ruid)
-    return;
-
-  int status = -1;
-
-  status = setfsuid(ruid);
-  if (status < 0) {
-    fprintf(stderr, "[ICFS] Couldn't set uid to %d.\n", euid);
-    exit(status);
-  }
-  pthread_mutex_unlock(&uid_switch);
-}
 
+/**
+ * Callback to validate the database schema matches expectations.
+ * Verifies column order, names, and types during schema checks.
+ *
+ * @param unused: Ignored context pointer
+ * @param argc: Number of columns returned (should be 3)
+ * @param argv: Column data: index 0=name, 1=type, 2=notnull flag
+ * @param colname: Column names (unused)
+ * @return 0 on success, 1 if schema mismatch
+ */
 static int check_table_col_schema(void *notused, int argc, char **argv,
                                   char **colname) {
   (void)notused;
@@ -86,27 +68,13 @@ static int check_table_col_schema(void *notused, int argc, char **argv,
   return 1;
 }
 
-static int set_flag(void *flag, int argc, char **argv, char **colname) {
+/**
-  (void)colname;
+ * Creates the permissions table schema in the database.
-
+ * Called when the table doesn't exist yet.
-  if (argc < 3) {
+ *
-    fprintf(
+ * @return 0 on success, 1 on failure
-        stderr,
+ */
-        "[ICFS] Unexpected amount of arguments given to the callback: %d.\n",
+int create_database_schema(void) {
-        argc);
-    return 1;
-  }
-
-  if (atoi(argv[2])) {
-    fprintf(stderr, "[ICFS] Third column was: %s\n", argv[2]);
-    *(int *)flag = 1;
-  } else {
-    *(int *)flag = -1;
-  }
-  return 0;
-}
-
-int create_database_schema() {
   fprintf(stderr, "[ICFS] Creating table 'permissions'.\n");
   const char *create_query =
       "CREATE TABLE permissions(executable TEXT NOT "
@@ -125,11 +93,12 @@ int create_database_schema() {
 }
 
 /**
- * Ensures that the database schema is correct.
+ * Ensures the database schema matches expected structure.
+ * Validates table existence and column definitions.
  *
- * @return: 0 if the schema is correct, 1 if the schema could not be corrected.
+ * @return 0 if schema is valid, 1 if validation failed or repair failed
  */
-int ensure_database_schema() {
+int ensure_database_schema(void) {
   // Check for the table.
   int result = sqlite3_table_column_metadata(
       perm_database, NULL, table_name, NULL, NULL, NULL, NULL, NULL, NULL);
@@ -145,6 +114,7 @@ int ensure_database_schema() {
     return 1;
   }
 
+  // Verify column definitions
   const char *pragma = "PRAGMA table_info(permissions);";
   char *err = NULL;
   int ret =
@@ -161,18 +131,20 @@ int ensure_database_schema() {
 }
 
 /**
- * Initializes the permanent permissions table.
+ * Initializes the permanent permissions database.
+ * Creates/opens the database file and sets up schema.
  *
- * @param db_filename: The filename of the permissions sqlite3 database
+ * @param db_filename Path to the SQLite database file
- * @return: 0 on success, -1 on failure
+ * @return 0 on success, -1 on failure
  */
 int init_perm_permissions_table(const char *db_filename) {
-  // we don't want the group and others to access the db
+  // Prevent group/others access during file creation
   umask(0077);
   ruid = getuid();
   euid = geteuid();
   fprintf(stderr, "[ICFS] Running with uid: %d, gid: %d\n", euid, getegid());
 
+  // Open database with read/write access and full mutex protection
   if (sqlite3_open_v2(db_filename, &perm_database,
                       SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE |
                           SQLITE_OPEN_FULLMUTEX,
@@ -180,12 +152,16 @@ int init_perm_permissions_table(const char *db_filename) {
     perror("[ICFS] Can't open permanent permissions database");
     return -1;
   }
-  umask(0);
+  umask(0); // Restore default umask
+
+  // Verify and initialize schema
   if (ensure_database_schema()) {
     fprintf(stderr, "[ICFS] Database schema is not correct.\n");
     return -1;
   }
 
+  // Switch to real UID, since we started with the icfs user to open the
+  // database
   int status = seteuid(ruid);
   if (status < 0) {
     fprintf(stderr,
@@ -197,17 +173,18 @@ int init_perm_permissions_table(const char *db_filename) {
 }
 
 /**
- * Destroys the permanent permissions table.
+ * Closes the database connection and releases resources.
+ * Should be called when the module is shutting down.
  */
 void destroy_perm_permissions_table(void) { sqlite3_close(perm_database); }
 
 /**
- * Checks if the process has a permanent access to the file.
+ * Checks if a specific process has permanent access to a file.
+ * Does not check parent processes.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename Path to the file being accessed
- * @param pi: The process information
+ * @param pi Process information structure
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * @return ALLOW (explicit allow), DENY (explicit deny), or NDEF (no info)
- * found
  */
 access_t check_perm_access_noparent(const char *filename,
                                     struct process_info pi) {
@@ -216,10 +193,16 @@ access_t check_perm_access_noparent(const char *filename,
 
   access_t ret = NDEF;
   sqlite3_stmt *stmt = NULL;
+
+  // Query checks:
+  // 1. Exact match for filename
+  // 2. Directory prefix match (filename starts with stored path + '/')
+  // Ordered by longest path for most specific match first
   const char *sql =
       "SELECT mode FROM permissions WHERE executable = ?1 "
       "AND (( ?2 LIKE (filename || \'%\') AND filename "
       "GLOB \'*/\') OR filename = ?2 ) ORDER BY LENGTH( filename ) DESC;";
+
   sqlite3_prepare_v2(perm_database, sql, -1, &stmt, NULL);
   sqlite3_bind_text(stmt, 1, pi.name, -1, SQLITE_STATIC);
   sqlite3_bind_text(stmt, 2, filename, -1, SQLITE_STATIC);
@@ -230,30 +213,24 @@ access_t check_perm_access_noparent(const char *filename,
     sqlite3_finalize(stmt);
     return ret;
   }
+
   if (step_ret == SQLITE_ROW) {
     int mode_col = sqlite3_column_int(stmt, 0);
-    if (mode_col) {
+    ret = mode_col ? ALLOW : DENY;
-      ret = ALLOW;
-    } else {
-      ret = DENY;
-    }
   }
-  sqlite3_finalize(stmt);
 
+  sqlite3_finalize(stmt);
   return ret;
 }
 
 /**
- * Checks if the process or any of it's parents have permanent access to the
+ * Checks if a process or any of its ancestors have permanent access.
- * file.
+ * Handles hierarchical permission inheritance.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename Path to the file being accessed
- * @param pi: The process information
+ * @param pi Process information structure
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * @return ALLOW/DENY/NDEF with NDEF meaning no explicit rule found
- * found. Does not return ALLOW_TEMP or DENY_TEMP.
+ * @note May return false negatives if parent processes terminate during check
- * @note: In case one of the parent processes is killed while this function
- * execution the result is not guranteed to be correct. It should only lead to
- * false negatives, though.
  */
 access_t check_perm_access(const char *filename, struct process_info pi) {
   if (pi.PID == 0 || pi.name == NULL) {
@@ -262,13 +239,17 @@ access_t check_perm_access(const char *filename, struct process_info pi) {
 
   struct process_info current_pi = pi;
   current_pi.name = strdup(current_pi.name);
+
   while (current_pi.PID != 0) {
     access_t access = check_perm_access_noparent(filename, current_pi);
     free(current_pi.name);
+
     if (access != NDEF) {
       return access;
     }
+
     current_pi.name = NULL;
+    // Traverse to parent process
     while (current_pi.name == NULL) {
       current_pi.PID = get_main_thread_pid(get_parent_pid(current_pi.PID));
       if (current_pi.PID != 0) {
@@ -283,11 +264,13 @@ access_t check_perm_access(const char *filename, struct process_info pi) {
 }
 
 /**
- * Gives permanent access to the process to the file.
+ * Sets a permanent access rule for a process-file combination.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename Path to the file needing access
- * @param pi: The process information
+ * @param pi Process information
- * @return: 0 on success, 1 on failure
+ * @param mode Kind of access rule to be set - SET_DENY to deny access, and
+ * SET_ALLOW to allow access.
+ * @return 0 on success, 1 on failure
  */
 int set_perm_access(const char *filename, struct process_info pi,
                     set_mode_t mode) {
@@ -299,19 +282,20 @@ int set_perm_access(const char *filename, struct process_info pi,
   } else if (mode == SET_DENY) {
     sql = "INSERT INTO permissions VALUES (?1, ?2, FALSE);";
   } else {
-    return 1;
+    return 1; // Invalid mode
   }
 
   sqlite3_prepare_v2(perm_database, sql, -1, &stmt, NULL);
   sqlite3_bind_text(stmt, 1, pi.name, -1, SQLITE_STATIC);
   sqlite3_bind_text(stmt, 2, filename, -1, SQLITE_STATIC);
+
   int step_ret = sqlite3_step(stmt);
   if (step_ret != SQLITE_DONE) {
     fprintf(stderr, "[ICFS] SQLite error: %s\n", sqlite3_errstr(step_ret));
     sqlite3_finalize(stmt);
     return 1;
   }
-  sqlite3_finalize(stmt);
 
+  sqlite3_finalize(stmt);
   return 0;
 }

src/perm_permissions_table.h

@@ -14,36 +14,38 @@
 #include "set_mode_t.h"
 
 /**
- * Initializes the permanent permissions table.
+ * Initializes the permanent permissions database.
+ * Creates/opens the database file and sets up schema.
  *
- * @param db_filename: The filename of the permissions sqlite3 database
+ * @param db_filename Path to the SQLite database file
- * @return: 0 on success, -1 on failure (e.g. ENOMEM)
+ * @return 0 on success, -1 on failure
  */
 int init_perm_permissions_table(const char *db_filename);
 
 /**
- * Destroys the permanent permissions table.
+ * Closes the database connection and releases resources.
+ * Should be called when the module is shutting down.
  */
 void destroy_perm_permissions_table();
 
 /**
- * Checks if the process has a permanent access to the file.
+ * Checks if a process or any of its ancestors have permanent access.
+ * Handles hierarchical permission inheritance.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename Path to the file being accessed
- * @param pi: The process information
+ * @param pi Process information structure
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * @return ALLOW/DENY/NDEF with NDEF meaning no explicit rule found
- * found
+ * @note May return false negatives if parent processes terminate during check
  */
 access_t check_perm_access(const char *filename, struct process_info pi);
 
 /**
- * Gives permanent access to the process to the file.
+ * Sets a permanent access rule for a process-file combination.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename Path to the file needing access
- * @param pi: The process information
+ * @param pi Process information
- * @param mode: Kind of access rule to be set - SET_DENY to deny access, and
+ * @param mode SET_ALLOW (whitelist) or SET_DENY (blacklist)
- * SET_ALLOW to allow access.
+ * @return 0 on success, 1 on failure
- * @return: 0 on success, 1 on failure
  */
 int set_perm_access(const char *filename, struct process_info pi,
                     set_mode_t mode);

src/proc_operations.c

@@ -5,7 +5,6 @@
   This program can be distributed under the terms of the GNU GPLv2.
   See the file LICENSE.
 */
-
 #include "proc_operations.h"
 #include <linux/limits.h>
 #include <stddef.h>
@@ -17,13 +16,19 @@
  * @brief Returns the PID of the main thread (i.e., the process ID) of the
  * process that the given thread ID (tid) belongs to.
  *
+ * In Linux, threads within the same process share the same thread group ID
+ * (TGID), which is equal to the PID of the main thread. This function retrieves
+ * the TGID by parsing `/proc/<tid>/status`, effectively returning the main
+ * thread's PID.
+ *
  * @param tid The thread ID (TID) of any thread in the process.
- * @return pid_t The process ID (main thread's PID), or -1 on error.
+ * @return pid_t The process ID (main thread's PID), or 0 on error (invalid tid
+ * or file read failure).
  */
 pid_t get_main_thread_pid(pid_t tid) {
   // Validate input
   if (tid <= 0) {
-    // errno = EINVAL;
+    // Invalid TID: TIDs are always positive in Linux
     return 0;
   }
 
@@ -45,14 +50,31 @@ pid_t get_main_thread_pid(pid_t tid) {
   }
 
   fclose(fp);
+
+  // Debugging check: If the provided tid is not the main thread's tid,
+  // this logs a message but does not affect the return value.
+  // This is useful for detecting cases where non-main threads are being
+  // inspected.
   if (tgid != tid) {
     fprintf(stderr,
             "[ICFS] The tid and and pid wasn't equal. tid:%d, pid:%d.\n", tid,
             tgid);
   }
+
   return tgid;
 }
 
+/**
+ * @brief Retrieves the full path of the executable for a given process ID.
+ *
+ * This function reads the `/proc/<pid>/exe` symbolic link, which points to the
+ * executable file of the process. The returned string is dynamically allocated
+ * and must be freed by the caller.
+ *
+ * @param pid The process ID to query.
+ * @return char* Dynamically allocated string containing the executable path, or
+ * NULL on failure.
+ */
 char *get_process_name_by_pid(const int pid) {
   char path[1024];
   sprintf(path, "/proc/%d/exe", pid);
@@ -74,6 +96,8 @@ char *get_process_name_by_pid(const int pid) {
       free(name);
       return NULL;
     }
+
+    // If the buffer was too small, double its size and try again
     if ((size_t)len >= size) {
       size *= 2;
       char *new_name = realloc(name, size);
@@ -83,8 +107,8 @@ char *get_process_name_by_pid(const int pid) {
       }
       name = new_name;
     } else {
-      // readlink does not set the null character
+      // readlink does not null-terminate, so we must do it manually
-      name[len] = 0;
+      name[len] = '\0';
       break;
     }
   }
@@ -93,16 +117,21 @@ char *get_process_name_by_pid(const int pid) {
 }
 
 /**
- * Finds the parent process ID of a given process.
+ * @brief Finds the parent process ID of a given process.
  *
- * @param pid: The process ID of the process to find the parent of
+ * This function parses the `/proc/<pid>/status` file to extract the `PPid`
- * @return: The parent process ID, or 0 if the parent process ID could not be
+ * field, which represents the parent process ID. This is useful for tracing
- * found
+ * process lineage.
+ *
+ * @param pid The process ID of the process to find the parent of.
+ * @return pid_t The parent process ID, or 0 if the parent could not be
+ * determined.
  */
 pid_t get_parent_pid(pid_t pid) {
   pid_t ppid = 0;
   char path[256];
-  snprintf(path, sizeof(path), "/proc/%u/status", pid);
+  snprintf(path, sizeof(path), "/proc/%d/status",
+           pid); // Use %d for signed pid_t
 
   FILE *file = fopen(path, "r");
   if (file == NULL) {
@@ -112,6 +141,7 @@ pid_t get_parent_pid(pid_t pid) {
 
   char line[256];
   while (fgets(line, sizeof(line), file)) {
+    // The "PPid:" field in `/proc/<pid>/status` is followed by a tab character
     if (sscanf(line, "PPid:\t%d", &ppid) == 1) {
       fclose(file);
       return ppid;

src/proc_operations.h

@@ -11,14 +11,29 @@
 
 #include <time.h>
 
+/**
+ * @brief Retrieves the full path of the executable for a given process ID.
+ *
+ * This function reads the `/proc/<pid>/exe` symbolic link, which points to the
+ * executable file of the process. The returned string is dynamically allocated
+ * and must be freed by the caller.
+ *
+ * @param pid The process ID to query.
+ * @return char* Dynamically allocated string containing the executable path, or
+ * NULL on failure.
+ */
 char *get_process_name_by_pid(const int pid);
 
 /**
- * Finds the parent process ID of a given process.
+ * @brief Finds the parent process ID of a given process.
  *
- * @param pid: The process ID of the process to find the parent of
+ * This function parses the `/proc/<pid>/status` file to extract the `PPid`
- * @return: The parent process ID, or 0 if the parent process ID could not be
+ * field, which represents the parent process ID. This is useful for tracing
- * found
+ * process lineage.
+ *
+ * @param pid The process ID of the process to find the parent of.
+ * @return pid_t The parent process ID, or 0 if the parent could not be
+ * determined.
  */
 pid_t get_parent_pid(pid_t pid);
 
@@ -26,8 +41,14 @@ pid_t get_parent_pid(pid_t pid);
  * @brief Returns the PID of the main thread (i.e., the process ID) of the
  * process that the given thread ID (tid) belongs to.
  *
+ * In Linux, threads within the same process share the same thread group ID
+ * (TGID), which is equal to the PID of the main thread. This function retrieves
+ * the TGID by parsing `/proc/<tid>/status`, effectively returning the main
+ * thread's PID.
+ *
  * @param tid The thread ID (TID) of any thread in the process.
- * @return pid_t The process ID (main thread's PID), or -1 on error.
+ * @return pid_t The process ID (main thread's PID), or 0 on error (invalid tid
+ * or file read failure).
  */
 pid_t get_main_thread_pid(pid_t tid);
 

src/real_filename.h

@@ -9,7 +9,18 @@
 #ifndef REAL_FILENAME_H
 #define REAL_FILENAME_H
 
+/**
+ * @brief Build a real path to the file.
+ *
+ * @param filename Relative path within the filesystem
+ * @return Newly allocated absolute path (caller must free)
+ */
 const char *real_filename(const char *filename);
+
+/**
+ * @brief Get the current mount point path
+ * @return Absolute path to the filesystem root
+ */
 const char *get_mountpoint(void);
 
 #endif // !REAL_FILENAME_H

src/sourcefs.c

@@ -16,20 +16,45 @@
 #include <string.h>
 #include <unistd.h>
 
+/**
+ * @brief Global handle for the source filesystem implementation
+ *
+ * Stores the root directory information to enable relative path operations.
+ */
 static struct source_files_handle {
-  const char *mountpoint;
+  const char *mountpoint; // Absolute path to the mounted filesystem root
-  int root_fd;
+  int root_fd;            // File descriptor for the root directory (O_PATH)
 } handle;
 
+/**
+ * @brief Translate FUSE paths to real paths in the underlying filesystem
+ *
+ * FUSE passes paths starting with '/' (e.g., "/dir/file"), but we need to
+ * operate relative to our root_fd. This function converts:
+ * - "/" -> "." (current directory)
+ * - "/dir/file" -> "dir/file"
+ *
+ * @param filename Path from FUSE (always starts with '/')
+ * @return Relative path suitable for use with root_fd
+ */
 const char *source_filename_translate(const char *filename) {
   if (strcmp("/", filename) == 0) {
     return ".";
-  } else {
-    return filename + 1;
   }
+  return filename + 1; // Skip leading slash
 }
 
+/**
+ * @brief Initialize the source filesystem with a root directory
+ *
+ * Sets up the mount point and opens a protected file descriptor to the
+ * root directory for safe relative operations.
+ *
+ * @param root_path Absolute path to the physical root directory
+ * @return 0 on success, -1 on failure
+ */
 int source_init(const char *root_path) {
+  // Allocate memory for mount point path
   handle.mountpoint = malloc(strlen(root_path) + 1);
   if (handle.mountpoint == NULL) {
     perror("[ICFS] Malloc failed");
@@ -38,8 +63,9 @@ int source_init(const char *root_path) {
 
   strcpy(handle.mountpoint, root_path);
 
+  // Open root directory with O_PATH to prevent accidental reads/writes
+  // while maintaining a valid descriptor for relative operations
   int root_fd = open(root_path, O_PATH);
-
   if (root_fd == -1) {
     fprintf(stderr, "[ICFS] Could not initialize source file system at %s",
             root_path);
@@ -48,66 +74,115 @@ int source_init(const char *root_path) {
   }
 
   handle.root_fd = root_fd;
-
   return 0;
 }
 
-void source_destroy(void) { free(handle.mountpoint); }
+/**
+ * @brief Clean up resources used by the source filesystem
+ */
+void source_destroy(void) {
+  free(handle.mountpoint); // Free allocated mount point path
+}
 
+/**
+ * @brief Get the current mount point path
+ * @return Absolute path to the filesystem root
+ */
 const char *get_mountpoint(void) { return handle.mountpoint; }
 
+/**
+ * @brief Build a real path to the file.
+ *
+ * @param filename Relative path within the filesystem
+ * @return Newly allocated absolute path (caller must free)
+ */
 const char *real_filename(const char *filename) {
   const char *mountpoint = get_mountpoint();
-  // Calculate required length
   size_t len1 = strlen(mountpoint);
   size_t len2 = strlen(filename);
-  size_t total_len = len1 + len2;
 
-  // Allocate memory (+1 for null terminator)
+  // Allocate space for combined path + null terminator
-  char *result = malloc(total_len + 1);
+  char *result = malloc(len1 + len2 + 1);
   if (result == NULL) {
     fprintf(stderr, "[ICFS] Memory allocation failed");
     perror("");
     return NULL;
   }
 
-  // Copy strings
   strcpy(result, mountpoint);
   strcat(result, filename);
-
   return result;
 }
 
+/**
+ * @brief Create a directory in the filesystem
+ *
+ * Uses mkdirat() to safely create directories relative to root_fd,
+ * preventing race conditions from concurrent path modifications.
+ */
 int source_mkdir(const char *filename, mode_t mode) {
   const char *relative_filename = source_filename_translate(filename);
   return mkdirat(handle.root_fd, relative_filename, mode);
 }
 
+/**
+ * @brief Remove a file from the filesystem
+ *
+ * Uses unlinkat() with AT_REMOVEDIR flag to safely remove files relative
+ * to the root directory.
+ */
 int source_unlink(const char *filename) {
   const char *relative_filename = source_filename_translate(filename);
   return unlinkat(handle.root_fd, relative_filename, 0);
 }
 
+/**
+ * @brief Get file status information
+ *
+ * Uses fstatat() to retrieve metadata relative to root_fd. Follows symlinks
+ * by default (flags=0).
+ */
 int source_stat(const char *restrict filename, struct stat *restrict statbuf) {
   const char *relative_filename = source_filename_translate(filename);
   return fstatat(handle.root_fd, relative_filename, statbuf, 0);
 }
 
+/**
+ * @brief Remove an empty directory
+ *
+ * Uses unlinkat() with AT_REMOVEDIR flag to safely remove directories.
+ */
 int source_rmdir(const char *filename) {
   const char *relative_filename = source_filename_translate(filename);
   return unlinkat(handle.root_fd, relative_filename, AT_REMOVEDIR);
 }
 
+/**
+ * @brief Create a symbolic link
+ *
+ * Creates symlinks relative to the root_fd directory for safety.
+ */
 int source_symlink(const char *target, const char *linkpath) {
   const char *relative_linkpath = source_filename_translate(linkpath);
   return symlinkat(target, handle.root_fd, relative_linkpath);
 }
 
+/**
+ * @brief Check file access permissions
+ *
+ * Uses faccessat() to check access rights relative to root_fd.
+ */
 int source_access(const char *filename, int mode) {
   const char *relative_filename = source_filename_translate(filename);
   return faccessat(handle.root_fd, relative_filename, mode, 0);
 }
 
+/**
+ * @brief Open a directory for reading
+ *
+ * Combines openat() and fdopendir() to safely access directories relative
+ * to the root_fd.
+ */
 DIR *source_opendir(const char *filename) {
   const char *relative_filename = source_filename_translate(filename);
   int fd = openat(handle.root_fd, relative_filename, 0);
@@ -119,6 +194,11 @@ DIR *source_opendir(const char *filename) {
   return dir_pointer;
 }
 
+/**
+ * @brief Rename a file or directory
+ *
+ * Uses renameat() to safely rename within the same root_fd namespace.
+ */
 int source_rename(const char *oldpath, const char *newpath) {
   const char *relative_oldpath = source_filename_translate(oldpath);
   const char *relative_newpath = source_filename_translate(newpath);
@@ -126,25 +206,48 @@ int source_rename(const char *oldpath, const char *newpath) {
                   relative_newpath);
 }
 
+/**
+ * @brief Create a hard link
+ *
+ * Uses linkat() with flags=0 (default behavior). May need AT_SYMLINK_NOFOLLOW
+ * if symlink handling should be modified.
+ */
 int source_link(const char *oldpath, const char *newpath) {
   const char *relative_oldpath = source_filename_translate(oldpath);
   const char *relative_newpath = source_filename_translate(newpath);
   return linkat(handle.root_fd, relative_oldpath, handle.root_fd,
                 relative_newpath, 0);
-  // NOTE: perhaps the flags here need to be reevaluated.
 }
 
+/**
+ * @brief Change file access mode
+ *
+ * Uses fchmodat() with flags=0 (follow symlinks). Consider using
+ * AT_SYMLINK_NOFOLLOW if symlink metadata should be modified directly.
+ */
 int source_chmod(const char *filename, mode_t mode) {
   const char *relative_filename = source_filename_translate(filename);
   return fchmodat(handle.root_fd, relative_filename, mode, 0);
-  // NOTE: perhaps the flags here need to be reevaluated.
 }
 
+/**
+ * @brief Change file owner and group
+ *
+ * Uses fchownat() with AT_SYMLINK_NOFOLLOW to modify symlink metadata
+ * rather than its target.
+ */
 int source_chown(const char *filename, uid_t owner, gid_t group) {
   const char *relative_filename = source_filename_translate(filename);
   return fchownat(handle.root_fd, filename, owner, group, AT_SYMLINK_NOFOLLOW);
 }
 
+/**
+ * @brief Truncate a file to a specified length
+ *
+ * Opens the file with read-only access then truncates it. This may fail
+ * if the file wasn't opened with write permissions. Consider changing
+ * openat() flags to O_WRONLY for reliability.
+ */
 int source_truncate(const char *filename, off_t length) {
   const char *relative_filename = source_filename_translate(filename);
   int fd = openat(handle.root_fd, relative_filename, 0);
@@ -155,12 +258,22 @@ int source_truncate(const char *filename, off_t length) {
   return ftruncate(fd, length);
 }
 
+/**
+ * @brief Open a file with specified flags
+ *
+ * Uses openat() to safely access files relative to root_fd.
+ */
 int source_open(const char *filename, int flags) {
   const char *relative_filename = source_filename_translate(filename);
   return openat(handle.root_fd, relative_filename, flags);
 }
 
+/**
+ * @brief Create and open a new file
+ *
+ * Uses openat() with O_CREAT to create files relative to root_fd.
+ */
 int source_create(const char *filename, int flags, mode_t mode) {
   const char *relative_filename = source_filename_translate(filename);
-  return openat(handle.root_fd, relative_filename, flags, mode);
+  return openat(handle.root_fd, relative_filename, flags | O_CREAT, mode);
 }

src/sourcefs.h

@@ -13,48 +13,129 @@
 #include <sys/stat.h>
 
 /**
- * Initializes the source file handling.
+ * @brief Initialize the source filesystem with a root directory
  *
- * @param root_path The root of the source files folder.
+ * Sets up the mount point and opens a protected file descriptor to the
- * @return 0 on success, -1 on failure.
+ * root directory for safe relative operations.
+ *
+ * @param root_path Absolute path to the physical root directory
+ * @return 0 on success, -1 on failure
  */
 int source_init(const char *root_path);
+
+/**
+ * @brief Clean up resources used by the source filesystem
+ */
 void source_destroy(void);
 
-/* All of the functions below are designed to behave exactly as their non-source
+/**
- * counterparts. */
+ * @brief Get file status information
-
+ *
+ * Uses fstatat() to retrieve metadata relative to root_fd. Follows symlinks
+ * by default (flags=0).
+ */
 int source_stat(const char *restrict filename, struct stat *restrict statbuf);
 
-struct dirent *source_readdir(DIR *dirp);
+/**
-
+ * @brief Open a directory for reading
+ *
+ * Combines openat() and fdopendir() to safely access directories relative
+ * to the root_fd.
+ */
 DIR *source_opendir(const char *filename);
 
+/**
+ * @brief Remove a file from the filesystem
+ *
+ * Uses unlinkat() with AT_REMOVEDIR flag to safely remove files relative
+ * to the root directory.
+ */
 int source_unlink(const char *filename);
 
+/**
+ * @brief Create a directory in the filesystem
+ *
+ * Uses mkdirat() to safely create directories relative to root_fd,
+ * preventing race conditions from concurrent path modifications.
+ */
 int source_mkdir(const char *filename, mode_t mode);
 
+/**
+ * @brief Remove an empty directory
+ *
+ * Uses unlinkat() with AT_REMOVEDIR flag to safely remove directories.
+ */
 int source_rmdir(const char *filename);
 
+/**
+ * @brief Create a symbolic link
+ *
+ * Creates symlinks relative to the root_fd directory for safety.
+ */
 int source_symlink(const char *target, const char *linkpath);
 
+/**
+ * @brief Rename a file or directory
+ *
+ * Uses renameat() to safely rename within the same root_fd namespace.
+ */
 int source_rename(const char *oldpath, const char *newpath);
 
+/**
+ * @brief Create a hard link
+ *
+ * Uses linkat() with flags=0 (default behavior). May need AT_SYMLINK_NOFOLLOW
+ * if symlink handling should be modified.
+ */
 int source_link(const char *oldpath, const char *newpath);
 
+/**
+ * @brief Change file access mode
+ *
+ * Uses fchmodat() with flags=0 (follow symlinks). Consider using
+ * AT_SYMLINK_NOFOLLOW if symlink metadata should be modified directly.
+ */
 int source_chmod(const char *filename, mode_t mode);
 
+/**
+ * @brief Change file owner and group
+ *
+ * Uses fchownat() with AT_SYMLINK_NOFOLLOW to modify symlink metadata
+ * rather than its target.
+ */
 int source_chown(const char *filename, uid_t owner, gid_t group);
 
+/**
+ * @brief Truncate a file to a specified length
+ *
+ * Opens the file with read-only access then truncates it. This may fail
+ * if the file wasn't opened with write permissions. Consider changing
+ * openat() flags to O_WRONLY for reliability.
+ */
 int source_truncate(const char *filename, off_t length);
 
+/**
+ * @brief Check file access permissions
+ *
+ * Uses faccessat() to check access rights relative to root_fd.
+ */
 int source_access(const char *filename, int mode);
 
 /* `open` and `create` are designed to correspond to fuse operations, not the
  * libc's `open(2)`. Both of them actually call `openat`. */
 
+/**
+ * @brief Open a file with specified flags
+ *
+ * Uses openat() to safely access files relative to root_fd.
+ */
 int source_open(const char *filename, int flags);
 
+/**
+ * @brief Create and open a new file
+ *
+ * Uses openat() with O_CREAT to create files relative to root_fd.
+ */
 int source_create(const char *filename, int flags, mode_t mode);
 
 #endif // !SOURCEFS_H

src/temp_permissions_table.c

@@ -19,26 +19,61 @@
 #include <time.h>
 #include <unistd.h>
 
+#define GC_INTERVAL 1 // Garbage collection time interval.
+
+/**
+ * @struct temp_process_permissions
+ * @brief Stores temporary file access permissions for a process
+ *
+ * This structure tracks temporary file access permissions for a specific
+ * process. The permissions are tied to the process's lifetime using its
+ * creation time.
+ */
 struct temp_process_permissions {
-  // yes, this is a correct type for start time in jiffies (see
+  unsigned long long creation_time; /**< Process creation time */
-  // proc_pid_stat(5))
+  vec(char *) allowed_files; /**< List of allowed file paths (prefixes) */
-  unsigned long long creation_time;
+  vec(char *) denied_files;  /**< List of denied file paths (prefixes) */
-  vec(char *) allowed_files;
-  vec(char *) denied_files;
 };
 
+/**
+ * @brief Global table mapping PIDs to temporary permissions
+ *
+ * This map stores the temporary file access permissions for processes.
+ * Entries are keyed by process ID (pid_t) and contain temp_process_permissions
+ * structures with the access rules.
+ */
 map(pid_t, struct temp_process_permissions) temp_permissions_table;
+
+/**
+ * @brief Read-write lock for thread-safe access to temp_permissions_table
+ *
+ * A read-write lock to ensure thread-safe operations on the
+ * temp_permissions_table. Allows concurrent reads but exclusive writes.
+ */
 pthread_rwlock_t temp_permissions_table_lock = PTHREAD_RWLOCK_INITIALIZER;
+
+/**
+ * @brief Thread handle for the garbage collector
+ *
+ * This thread handles cleanup of stale entries in the temp_permissions_table.
+ */
 pthread_t gc_thread;
+
+/**
+ * @brief Flag indicating whether garbage collector is active
+ *
+ * When non-zero, indicates that the garbage collector thread should continue
+ * running.
+ */
 int is_gc_active = 0;
 
 /**
- * Function to get the process creation time (in jiffies) from the proc
+ * Function to get the process creation time from the proc
  * filesystem
  *
- * @param pid: The process ID of the process to get the creation time of
+ * @param pid The process ID of the process to get the creation time of
- * @return: The process creation time in jiffies, or 0 on error
+ * @return The process creation time, or 0 on error
- * @note: although nothing in the documentation says that the creation time is
+ * @note although nothing in the documentation says that the creation time is
  * never really equal to 0, it exceptionally unlikely.
  */
 unsigned long long get_process_creation_time(pid_t pid) {
@@ -77,12 +112,23 @@ unsigned long long get_process_creation_time(pid_t pid) {
   return creation_time;
 }
 
+/**
+ * @brief Validates if a process entry is still valid
+ *
+ * Checks if the given process entry matches the current process state
+ * by comparing creation times.
+ *
+ * @param pid Process ID to validate
+ * @param entry Pointer to the permission entry to validate
+ * @return 1 if valid, 0 if invalid or error
+ */
 int is_valid(pid_t pid, struct temp_process_permissions *entry) {
   unsigned long long creation_time = get_process_creation_time(pid);
   if (creation_time == 0) {
     return 0;
   }
 
+  // If the creation time doesn't match, this is a different process instance
   if (creation_time != entry->creation_time) {
     return 0;
   }
@@ -90,19 +136,30 @@ int is_valid(pid_t pid, struct temp_process_permissions *entry) {
   return 1;
 }
 
+/**
+ * @brief Garbage collector thread for cleaning up stale entries
+ *
+ * Periodically scans the permissions table and removes entries for processes
+ * that no longer exist. Runs every second while is_gc_active is true.
+ *
+ * @param arg Unused thread argument parameter
+ * @return Always NULL
+ */
 void *garbage_collector(void *arg) {
   (void)arg;
 
   while (is_gc_active) {
-    sleep(1);
+    sleep(GC_INTERVAL); // Check once per second for stale entries
     pthread_rwlock_wrlock(&temp_permissions_table_lock);
 
     vec(pid_t) blacklist;
     init(&blacklist);
 
+    // Identify stale entries
     for_each(&temp_permissions_table, pid, entry) {
       if (!is_valid(*pid, entry)) {
         push(&blacklist, *pid);
+        // Free memory for this entry's file lists
         for_each(&entry->allowed_files, allowed_file) { free(*allowed_file); }
         cleanup(&entry->allowed_files);
         for_each(&entry->denied_files, denied_file) { free(*denied_file); }
@@ -110,6 +167,7 @@ void *garbage_collector(void *arg) {
       }
     }
 
+    // Remove all stale entries from the table
     for_each(&blacklist, pid) { erase(&temp_permissions_table, *pid); }
 
     cleanup(&blacklist);
@@ -123,7 +181,7 @@ void *garbage_collector(void *arg) {
 /**
  * Initializes the temporary permissions table.
  *
- * @return: 0 on success, -1 on failure (e.g. ENOMEM)
+ * @return 0 on success, -1 on failure
  */
 int init_temp_permissions_table(void) {
   init(&temp_permissions_table);
@@ -132,7 +190,7 @@ int init_temp_permissions_table(void) {
 /**
  * Starts the temporary permissions table garbage_collector.
  *
- * @return: 0 on success, -1 on failure (e.g. ENOMEM)
+ * @return 0 on success, -1 on failure
  */
 int init_garbage_collector(void) {
   is_gc_active = 1;
@@ -145,7 +203,7 @@ int init_garbage_collector(void) {
 /**
  * Destroys the temporary permissions table.
  *
- * @note: the table is guranteed to be destroyed if it is already initialized.
+ * @note the table is guranteed to be destroyed if it is already initialized.
  * It does not indicate any errors whatsoever. If something goes wrong - you are
  * screwed.
  */
@@ -155,7 +213,7 @@ void destroy_temp_permissions_table(void) {
     pthread_join(gc_thread, NULL);
   }
 
-  // free the memory allocated for the table
+  // Free the memory allocated for the table
   for_each(&temp_permissions_table, entry) {
     for_each(&entry->allowed_files, allowed_file) { free(*allowed_file); }
     cleanup(&entry->allowed_files);
@@ -169,15 +227,17 @@ void destroy_temp_permissions_table(void) {
 }
 
 /**
- * Checks if the process has a temporary access to the file.
+ * @brief Checks if a specific process has temporary access to a file
  *
- * @param filename: The file that the process is trying to access
+ * Checks only the specified process (not its parents) for temporary access
- * @param pid: PID of the process
+ * permissions to the given file. Uses a longest-match algorithm for path
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * prefixes.
- * found is avaliable
+ *
+ * @param filename Path to the file being accessed
+ * @param pid Process ID of the process to check
+ * @return Access status ALLOW, DENY, or NDEF (no information)
  */
 access_t check_temp_access_noparent(const char *filename, pid_t pid) {
-  // TODO: more efficient locking
   pthread_rwlock_rdlock(&temp_permissions_table_lock);
   struct temp_process_permissions *permission_entry =
       get(&temp_permissions_table, pid);
@@ -190,13 +250,15 @@ access_t check_temp_access_noparent(const char *filename, pid_t pid) {
     }
 
     if (process_creation_time == permission_entry->creation_time) {
-      // the process is the same as the one that was granted temporary access
+      // The process is the same as the one that was granted temporary access
-      // to the file
       size_t filename_len = strlen(filename);
       access_t ret = NDEF;
       size_t maxlen = 0;
+
+      // Check denied files first (deny takes precedence over allow)
       for_each(&permission_entry->denied_files, denied_file) {
         size_t denied_file_len = strlen(*denied_file);
+        // Check if this denied path prefix matches the requested file
         if ((strncmp(*denied_file, filename, denied_file_len) == 0 &&
              ((denied_file_len < filename_len &&
                (*denied_file)[denied_file_len - 1] == '/') ||
@@ -206,17 +268,21 @@ access_t check_temp_access_noparent(const char *filename, pid_t pid) {
           ret = DENY;
         }
       }
+
+      // Check allowed files
       for_each(&permission_entry->allowed_files, allowed_file) {
         size_t allowed_file_len = strlen(*allowed_file);
+        // Check if this allowed path prefix matches the requested file
         if ((strncmp(*allowed_file, filename, allowed_file_len) == 0 &&
              ((allowed_file_len < filename_len &&
                (*allowed_file)[allowed_file_len - 1] == '/') ||
               (allowed_file_len == filename_len))) &&
-            allowed_file > maxlen) {
+            allowed_file_len > maxlen) {
           maxlen = allowed_file_len;
           ret = ALLOW;
         }
       }
+
       pthread_rwlock_unlock(&temp_permissions_table_lock);
       return ret;
     }
@@ -229,12 +295,12 @@ access_t check_temp_access_noparent(const char *filename, pid_t pid) {
  * Checks if the process or any of it's parents have temporary access to the
  * file.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename The file that the process is trying to access
- * @param pi: The process information
+ * @param pi The process information
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * @return access status - ALLOW, DENY or NDEF in case if no information was
  * found. Does not return ALLOW_TEMP.
- * @note: In case one of the parent processes is killed while this function
+ * @note In case one of the parent processes is killed while this function
- * execution the result is not guranteed to be correct. It should only lead to
+ * execution the result is not guranteed to be correct. It should only
  * false negatives, though.
  */
 access_t check_temp_access(const char *filename, struct process_info pi) {
@@ -253,11 +319,11 @@ access_t check_temp_access(const char *filename, struct process_info pi) {
 /**
  * Sets temporary access mode of the process to the file.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename The file that the process is trying to access
- * @param pi: The process information
+ * @param pi The process information
- * @param mode: Kind of access rule to be set - SET_DENY to deny access, and
+ * @param mode Kind of access rule to be set - SET_DENY to deny access, and
  * SET_ALLOW to allow access.
- * @return: 0 on success, -1 on failure.
+ * @return 0 on success, -1 on failure.
  */
 int set_temp_access(const char *filename, struct process_info pi,
                     set_mode_t mode) {
@@ -278,8 +344,7 @@ int set_temp_access(const char *filename, struct process_info pi,
     }
 
     if (process_creation_time == permission_entry->creation_time) {
-      // the process is the same as the one that was granted temporary access
+      // The process is the same as the one that was granted temporary access
-      // to the file
       if (mode == SET_ALLOW) {
         push(&permission_entry->allowed_files, strdup(filename));
       }
@@ -290,19 +355,19 @@ int set_temp_access(const char *filename, struct process_info pi,
       pthread_rwlock_unlock(&temp_permissions_table_lock);
       return 0;
     }
-    // we have an entry for the process, but the process is different
+    // We have an entry for the process, but the process is different
-    // delete the entry and create a new one
+    // Delete the entry and create a new one
     erase(&temp_permissions_table, pi.PID);
     permission_entry = NULL;
   }
 
-  // no entry is present
+  // No entry is present - construct a new one
-  // construct the entry
   struct temp_process_permissions new_permission_entry;
 
   new_permission_entry.creation_time = get_process_creation_time(pi.PID);
   init(&new_permission_entry.allowed_files);
   init(&new_permission_entry.denied_files);
+
   if (mode == SET_ALLOW) {
     push(&new_permission_entry.allowed_files, strdup(filename));
   }

src/temp_permissions_table.h

@@ -16,21 +16,21 @@
 /**
  * Initializes the temporary permissions table.
  *
- * @return: 0 on success, -1 on failure (e.g. ENOMEM)
+ * @return: 0 on success, -1 on failure
  */
 int init_temp_permissions_table(void);
 
 /**
  * Starts the temporary permissions table garbage_collector.
  *
- * @return: 0 on success, -1 on failure (e.g. ENOMEM)
+ * @return 0 on success, -1 on failure
  */
 int init_garbage_collector(void);
 
 /**
  * Destroys the temporary permissions table.
  *
- * @note: the table is guranteed to be destroyed if it is already initialized.
+ * @note the table is guranteed to be destroyed if it is already initialized.
  * It does not indicate any errors whatsoever. If something goes wrong - you are
  * screwed.
  */
@@ -40,12 +40,12 @@ void destroy_temp_permissions_table(void);
  * Checks if the process or any of it's parents have temporary access to the
  * file.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename The file that the process is trying to access
- * @param pi: The process information
+ * @param pi The process information
- * @return: access status - ALLOW, DENY or NDEF in case if no information was
+ * @return access status - ALLOW, DENY or NDEF in case if no information was
  * found. Does not return ALLOW_TEMP.
- * @note: In case one of the parent processes is killed while this function
+ * @note In case one of the parent processes is killed while this function
- * execution the result is not guranteed to be correct. It should only lead to
+ * execution the result is not guranteed to be correct. It should only
  * false negatives, though.
  */
 access_t check_temp_access(const char *filename, struct process_info pi);
@@ -53,11 +53,11 @@ access_t check_temp_access(const char *filename, struct process_info pi);
 /**
  * Sets temporary access mode of the process to the file.
  *
- * @param filename: The file that the process is trying to access
+ * @param filename The file that the process is trying to access
- * @param pi: The process information
+ * @param pi The process information
- * @param mode: Kind of access rule to be set - SET_DENY to deny access, and
+ * @param mode Kind of access rule to be set - SET_DENY to deny access, and
  * SET_ALLOW to allow access.
- * @return: 0 on success, -1 on failure.
+ * @return 0 on success, -1 on failure.
  */
 int set_temp_access(const char *filename, struct process_info pi,
                     set_mode_t mode);

src/ui-socket.c

@@ -10,7 +10,7 @@
 #include <stddef.h>
 #include <sys/types.h>
 #include <time.h>
-#define _GNU_SOURCE
+#define _GNU_SOURCE // Required for certain POSIX extensions
 #include "cc.h"
 #include "perm_permissions_table.h"
 #include "real_filename.h"
@@ -26,34 +26,45 @@
 #include <sys/un.h>
 #include <unistd.h>
 
+// Exit status codes for icfs_dialogue process interaction
 #define DIALOGUE_YES 1
 #define DIALOGUE_NO 0
 #define DIALOGUE_PERM 2
-#define DIALOGUE_TEMP 0
+#define DIALOGUE_TEMP 0 // Bitmask position, not value
 
+// Mutex to protect concurrent access to permission tables
 pthread_mutex_t access_check_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+// Structure to hold user permission decision response
 struct dialogue_response {
   access_t decision;
   char *filename;
 };
 
+/**
+ * Initialize UI socket and required permission tables
+ *
+ * @param perm_permissions_db_filename Path to persistent permissions DB
+ * @return 0 on success, 1 on failure
+ */
 int init_ui_socket(const char *perm_permissions_db_filename) {
   FILE *fp = NULL;
 
+  // Initialize in-memory temporary permissions table
   if (init_temp_permissions_table()) {
     fprintf(stderr,
             "[ICFS] Could not initialize temporary permissions table.\n");
     return 1;
   }
 
+  // Initialize permanent permissions database
   if (init_perm_permissions_table(perm_permissions_db_filename)) {
     fprintf(stderr,
             "[ICFS] Could not initialize permanent permissions table.\n");
     return 1;
   }
 
-  // Test if dialogue is installed (get version)
+  // Verify dialogue utility is available
   fp = popen("icfs_dialogue --version", "r");
   if (fp == NULL) {
     perror("[ICFS] Pipe returned an error");
@@ -64,19 +75,23 @@ int init_ui_socket(const char *perm_permissions_db_filename) {
   return 0;
 }
 
+/**
+ * Clean up UI socket resources
+ */
 void destroy_ui_socket(void) {
   destroy_temp_permissions_table();
   destroy_perm_permissions_table();
 }
 
 /**
- * Asks the user if the process should be allowed to access the file using the
+ * Query user for file access permission through GUI dialogue
- * GUI
  *
- * @param filename: The file that the process is trying to access
+ * Constructs and executes the icfs_dialogue command to get user consent.
- * @param pi: The process information
+ * Handles memory allocation errors gracefully and parses the response.
- * @return: access status - ALLOW, DENY or ALLOW_TEMP
+ *
- * allowed for the runtime of the process
+ * @param filename File being accessed
+ * @param proc_info Process requesting access
+ * @return Struct containing access decision and resolved filename
  */
 struct dialogue_response ask_access(const char *filename,
                                     struct process_info proc_info) {
@@ -90,26 +105,21 @@ struct dialogue_response ask_access(const char *filename,
   response.filename = NULL;
 
   if (ret < 0) {
-    // If asprintf fails, the contents of command are undefined (see man
+    // Memory allocation failed - create minimal fallback response
-    // asprintf). That does not explicitly rule out that command will be a valid
-    // pointer. But the risk of freeing a non-allocated pointer is too much to
-    // justify preparing for this.
     fprintf(stderr, "[ICFS] Could not create query on rule insertion");
     perror("");
-    response.decision = DENY;
     response.filename = malloc(2);
     response.filename[0] = '/';
     response.filename[1] = 0;
     return response;
   }
 
-  // dialogue Question Message Popup
+  // Execute permission dialogue
   fp = popen(command, "r");
   free(command);
 
   if (fp == NULL) {
     perror("[ICFS] Pipe returned a error");
-    response.decision = DENY;
     response.filename = malloc(2);
     response.filename[0] = '/';
     response.filename[1] = 0;
@@ -119,9 +129,9 @@ struct dialogue_response ask_access(const char *filename,
   str(char) dialogue_output;
   init(&dialogue_output);
 
-  char line[1024]; // Buffer to read individual lines
+  char line[1024];
 
-  // Read the command output line by line
+  // Read entire command output
   while (fgets(line, sizeof(line), fp)) {
     push_fmt(&dialogue_output, line);
   }
@@ -131,20 +141,20 @@ struct dialogue_response ask_access(const char *filename,
   fprintf(stderr, "[ICFS] dialogue wrote out %s\n", first(&dialogue_output));
   fprintf(stderr, "[ICFS] dialogue returned %d\n", dialogue_exit_code);
 
+  // Handle empty output case
   if (size(&dialogue_output) == 0) {
     push(&dialogue_output, '/');
   }
 
+  // Validate string length consistency
   assert(strlen(first(&dialogue_output)) == size(&dialogue_output));
 
+  // Allocate and copy final filename
   response.filename = malloc(size(&dialogue_output) + 1);
   strcpy(response.filename, first(&dialogue_output));
-  // response.filename[size(&dialogue_output)] = 0;
-
-  // assert(0 == strcmp(response.filename, first(&dialogue_output)));
   cleanup(&dialogue_output);
-  time_t now = time(0);
 
+  // Parse exit code combination
   if (dialogue_exit_code == (DIALOGUE_YES | DIALOGUE_PERM)) {
     response.decision = ALLOW;
   } else if (dialogue_exit_code == DIALOGUE_YES | DIALOGUE_TEMP) {
@@ -159,68 +169,58 @@ struct dialogue_response ask_access(const char *filename,
 }
 
 /**
- * Check access according to:
+ * Determine file access based on permission tables and user input
- * 1. temp permission table
- * 2. permanent permission table
- * 3. user descision
  *
- * @param filename: The file that the process is trying to access
+ * Checks permissions in order:
- * @param pi: The process information
+ * 1. Temporary permission table
- * @param opts: options (GRANT_TEMP, GRANT_PERM)
+ * 2. Permanent permission table
- * @return: 0 if access is denied, 1 if access is allowed
+ * 3. User decision via GUI
+ *
+ * @param filename File being accessed
+ * @param pi Process information
+ * @param opts Flags to force permission (GRANT_TEMP/GRANT_PERM)
+ * @return 1 if access allowed, 0 if denied
  */
 int interactive_access(const char *filename, struct process_info proc_info,
                        int opts) {
   char *real_path = real_filename(filename);
   pthread_mutex_lock(&access_check_mutex);
 
+  // First check temporary permissions
   access_t access = check_temp_access(real_path, proc_info);
   if (access == ALLOW) {
-    fprintf(
+    fprintf(stderr, "[ICFS] Permission allowed to %s based on temp table.\n",
-        stderr,
+            proc_info.name);
-        "[ICFS] Permission allowed to %s based on a rule present in the temp "
-        "permission table.\n",
-        proc_info.name);
     free(real_path);
     pthread_mutex_unlock(&access_check_mutex);
     return 1;
   }
   if (access == DENY) {
-    fprintf(
+    fprintf(stderr, "[ICFS] Permission denied to %s based on temp table.\n",
-        stderr,
+            proc_info.name);
-        "[ICFS] Permission denied to %s based on a rule present in the temp "
-        "permission table.\n",
-        proc_info.name);
     free(real_path);
     pthread_mutex_unlock(&access_check_mutex);
     return 0;
   }
 
+  // Then check permanent permissions
   access = check_perm_access(real_path, proc_info);
   if (access == ALLOW) {
-    fprintf(
+    fprintf(stderr, "[ICFS] Permission allowed to %s based on perm table.\n",
-        stderr,
+            proc_info.name);
-        "[ICFS] Permission allowed to %s based on a rule present in the perm "
-        "permission table.\n",
-        proc_info.name);
     free(real_path);
     pthread_mutex_unlock(&access_check_mutex);
     return 1;
   }
   if (access == DENY) {
-    fprintf(
+    fprintf(stderr, "[ICFS] Permission denied to %s based on perm table.\n",
-        stderr,
+            proc_info.name);
-        "[ICFS] Permission denied to %s based on a rule present in the perm "
-        "permission table.\n",
-        proc_info.name);
     free(real_path);
     pthread_mutex_unlock(&access_check_mutex);
     return 0;
   }
 
-  // if noth GRANT_TEMP and GRANT_PERM are selected, then only permanent
+  // Handle forced permission grants
-  // permissions are granted
-
   if (opts & GRANT_PERM) {
     fprintf(stderr, "[ICFS] Permission granted permanently to %s.\n",
             proc_info.name);
@@ -238,66 +238,59 @@ int interactive_access(const char *filename, struct process_info proc_info,
     return 1;
   }
 
+  // Get user decision
   struct dialogue_response response = ask_access(filename, proc_info);
-  // fprintf(stderr, "%s", response.filename);
-  // assert(0 != strlen(response.filename));
-
-  // the user might specify a different file in the dialogue, so we need to
-  // check if it is valid
 
+  // Validate returned filename meets requirements:
+  // 1. Must exist
+  // 2. Must be prefix of original filename with trailing slash
+  //    or exact match
   while (
       source_access(response.filename, F_OK) ||
       !(strncmp(response.filename, filename, strlen(response.filename)) == 0 &&
         ((strlen(response.filename) < strlen(filename) &&
           response.filename[strlen(response.filename) - 1] == '/') ||
          (strlen(response.filename) == strlen(filename))))) {
-    // if it is invalid, just ask again.
+    fprintf(stderr, "[ICFS] Invalid filename returned by dialogue: %s\n",
-    fprintf(stderr,
-            "[ICFS] Filename returned by access dialogue wasn't correct: %s\n",
             response.filename);
     free(response.filename);
     response = ask_access(filename, proc_info);
   }
 
   free(real_path);
-
   real_path = real_filename(response.filename);
   free(response.filename);
 
   int ret = 0;
 
+  // Apply user decision to appropriate permission table
   if (response.decision == ALLOW) {
     fprintf(stderr,
-            "[ICFS] Permission granted permanently to %s based on zenty "
+            "[ICFS] Permission granted permanently to %s based on response.\n",
-            "response.\n",
             proc_info.name);
     set_perm_access(real_path, proc_info, SET_ALLOW);
     ret = 1;
   } else if (response.decision == ALLOW_TEMP) {
     fprintf(stderr,
-            "[ICFS] Permission granted temporarily to %s based on zenty "
+            "[ICFS] Permission granted temporarily to %s based on response.\n",
-            "response.\n",
             proc_info.name);
     set_temp_access(real_path, proc_info, SET_ALLOW);
     ret = 1;
   } else if (response.decision == DENY_TEMP) {
-    fprintf(
+    fprintf(stderr,
-        stderr,
+            "[ICFS] Permission denied temporarily to %s based on response.\n",
-        "[ICFS] Permission denied temporarily to %s based on zenty response.\n",
+            proc_info.name);
-        proc_info.name);
     set_temp_access(real_path, proc_info, SET_DENY);
     ret = 0;
   } else if (response.decision == DENY) {
-    fprintf(
+    fprintf(stderr,
-        stderr,
+            "[ICFS] Permission denied permanently to %s based on response.\n",
-        "[ICFS] Permission denied permanently to %s based on zenty response.\n",
+            proc_info.name);
-        proc_info.name);
     set_perm_access(real_path, proc_info, SET_DENY);
     ret = 0;
   }
-  pthread_mutex_unlock(&access_check_mutex);
 
+  pthread_mutex_unlock(&access_check_mutex);
   free(real_path);
-  // deny on unknown options.
   return ret;
 }

src/ui-socket.h

@@ -6,10 +6,6 @@
   See the file LICENSE.
 */
 
-/*
- * Interface for controlling communication with the UI.
- */
-
 #ifndef UI_SOCKET_H
 #define UI_SOCKET_H
 
@@ -17,27 +13,30 @@
 #include <sys/types.h>
 
 /**
- * Initialize the GUI communication.
+ * Initialize UI socket and required permission tables
  *
- * @return: 0 on success, -1 on faliure.
+ * @param perm_permissions_db_filename Path to persistent permissions DB
+ * @return 0 on success, 1 on failure
  */
 int init_ui_socket(const char *perm_permissions_db_filename);
 
 /**
- * Close the GUI communication.
+ * Clean up UI socket resources
  */
 void destroy_ui_socket(void);
 
 /**
- * Check access according to:
+ * Determine file access based on permission tables and user input
- * 1. temporary permission table
- * 2. permanent permission table
- * 3. user descision
  *
- * @param filename: The file that the process is trying to access
+ * Checks permissions in order:
- * @param pi: The process information
+ * 1. Temporary permission table
- * @param opts: options (GRANT_TEMP, GRANT_PERM)
+ * 2. Permanent permission table
- * @return: 0 if access is denied, 1 if access is allowed
+ * 3. User decision via GUI
+ *
+ * @param filename File being accessed
+ * @param pi Process information
+ * @param opts Flags to force permission (GRANT_TEMP/GRANT_PERM)
+ * @return 1 if access allowed, 0 if denied
  */
 int interactive_access(const char *filename, struct process_info pi, int opts);
 

test/test.bash

@@ -52,8 +52,7 @@ elif [[ $1 == "--performance" ]]; then
 else
   echo "Database protection will not be tested due to the lack of setuid capabilites."
   echo "To test it, run this script with '--setuid'."
-  #valgrind --leak-check=full -s ../build/icfs -o default_permissions -o debug ./protected ./.pt.db 2>&1 | grep "==\|zenity\|Permission\|column\|callback\|SQLite" &
+  valgrind --leak-check=full -s ../build/icfs -o default_permissions ./protected ./.pt.db &
-  valgrind --leak-check=full --show-leak-kinds=all -s ../build/icfs -o default_permissions ./protected ./.pt.db &
   sleep 5
 fi
 
@@ -213,14 +212,10 @@ else
   echo "[ICFS-TEST]: permanent permissions database access was not tested due to the lack of seuid bit setting capabilites. To test this, run the script with '--setuid' flag"
 fi
 
+# performance testing code
+
 RUNS_NUM=500
-
 if [[ $1 == '--performance' ]]; then
-
-  #warmup
-  icfs_dialogue --set-fake-response yes
-  parallel -j8 ::: "./stress.bash $RUNS_NUM openers/opener10"
-
   icfs_dialogue --set-fake-response yes
   echo "[ICFS-TEST]: temp permissions"
   time parallel -j8 ::: "./stress.bash $RUNS_NUM openers/opener7"
@@ -228,7 +223,6 @@ if [[ $1 == '--performance' ]]; then
   icfs_dialogue --set-fake-response yes_perm
   echo "[ICFS-TEST]: perm permissions"
   time parallel -j8 ::: "./stress.bash $RUNS_NUM openers/opener8"
-
 fi
 
 # unmount
@@ -238,11 +232,9 @@ sleep 0.5
 umount "$(realpath ./protected)"
 sleep 3
 
+# test the same thing, but without ICFS mounted
+
 if [[ $1 == '--performance' ]]; then
-
-  #warmup
-  parallel -j8 ::: "./stress.bash $RUNS_NUM openers/opener9"
-
   echo "[ICFS-TEST]: bare filesystem"
   time parallel -j8 ::: "./stress.bash $RUNS_NUM openers/opener9"
 fi