From 94d531fd827457cf71fe7f3808c7a2f531dceba5 Mon Sep 17 00:00:00 2001
From: NRK <nrk@disroot.org>
Date: Sat, 30 Jul 2022 10:05:10 +0200
Subject: [PATCH] autoreload: simplify and cleanup (#342)

the current code is quite hacky and complex as it mixes multiple pointers. all
of this complexity is unnecessary. drop it by introducing an explicit scratch
buffer instead of implicitly abusing `arl->filename` as one. this also reduces
some unnecessary allocation overhead.

additionally, the argument to arl_setup must be the result of `realpath(3)` (as
commented in `nsxiv.h`). instead of commenting it, assert it.

and lastly, rename `arl_setup` to `arl_add` since it's not doing any "setup"
but rather *adding* a file to watch.

Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/342
Reviewed-by: explosion-mental <explosion-mental@noreply.codeberg.org>
---
 autoreload.c | 41 ++++++++++++++++++++++++-----------------
 main.c       |  2 +-
 nsxiv.h      |  4 ++--
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/autoreload.c b/autoreload.c
index c4b5665..c9ad45d 100644
--- a/autoreload.c
+++ b/autoreload.c
@@ -1,4 +1,5 @@
 /* Copyright 2017 Max Voit, Bert Muennich
+ * Copyright 2022 nsxiv contributors
  *
  * This file is a part of nsxiv.
  *
@@ -20,16 +21,14 @@
 
 #if HAVE_INOTIFY
 
+#include <assert.h>
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/inotify.h>
 #include <unistd.h>
 
-static union {
-	char d[4096]; /* aligned buffer */
-	struct inotify_event e;
-} buf;
+static struct { char *buf; size_t len; } scratch;
 
 void arl_init(arl_t *arl)
 {
@@ -43,7 +42,7 @@ CLEANUP void arl_cleanup(arl_t *arl)
 {
 	if (arl->fd != -1)
 		close(arl->fd);
-	free(arl->filename);
+	free(scratch.buf);
 }
 
 static void rm_watch(int fd, int *wd)
@@ -61,26 +60,32 @@ static void add_watch(int fd, int *wd, const char *path, uint32_t mask)
 		error(0, errno, "inotify: %s", path);
 }
 
-void arl_setup(arl_t *arl, const char *filepath)
+static char *arl_scratch_push(const char *filepath, size_t len)
 {
-	char *base = strrchr(filepath, '/');
+	if (scratch.len < len + 1) {
+		scratch.len = len + 1;
+		scratch.buf = erealloc(scratch.buf, scratch.len);
+	}
+	scratch.buf[len] = '\0';
+	return memcpy(scratch.buf, filepath, len);
+}
+
+void arl_add(arl_t *arl, const char *filepath)
+{
+	char *base, *dir;
 
 	if (arl->fd == -1)
 		return;
 
 	rm_watch(arl->fd, &arl->wd_dir);
 	rm_watch(arl->fd, &arl->wd_file);
-
 	add_watch(arl->fd, &arl->wd_file, filepath, IN_CLOSE_WRITE | IN_DELETE_SELF);
 
-	free(arl->filename);
-	arl->filename = estrdup(filepath);
-
-	if (base != NULL) {
-		arl->filename[++base - filepath] = '\0';
-		add_watch(arl->fd, &arl->wd_dir, arl->filename, IN_CREATE | IN_MOVED_TO);
-		strcpy(arl->filename, base); /* NOLINT: basename will always be shorter than fullpath */
-	}
+	base = strrchr(filepath, '/');
+	assert(base != NULL); /* filepath must be result of `realpath(3)` */
+	dir = arl_scratch_push(filepath, base - filepath);
+	add_watch(arl->fd, &arl->wd_dir, dir, IN_CREATE | IN_MOVED_TO);
+	arl->filename = arl_scratch_push(base + 1, strlen(base + 1));
 }
 
 bool arl_handle(arl_t *arl)
@@ -88,6 +93,8 @@ bool arl_handle(arl_t *arl)
 	bool reload = false;
 	char *ptr;
 	const struct inotify_event *e;
+	/* inotify_event aligned buffer */
+	static union { char d[4096]; struct inotify_event e; } buf;
 
 	while (true) {
 		ssize_t len = read(arl->fd, buf.d, sizeof(buf.d));
@@ -124,7 +131,7 @@ void arl_cleanup(arl_t *arl)
 	(void) arl;
 }
 
-void arl_setup(arl_t *arl, const char *filepath)
+void arl_add(arl_t *arl, const char *filepath)
 {
 	(void) arl;
 	(void) filepath;
diff --git a/main.c b/main.c
index 87dfc14..12cf404 100644
--- a/main.c
+++ b/main.c
@@ -350,7 +350,7 @@ void load_image(int new)
 
 	close_info();
 	open_info();
-	arl_setup(&arl, files[fileidx].path);
+	arl_add(&arl, files[fileidx].path);
 	title_dirty = true;
 
 	if (img.multi.cnt > 0 && img.multi.animate)
diff --git a/nsxiv.h b/nsxiv.h
index 4f903e9..7262e1e 100644
--- a/nsxiv.h
+++ b/nsxiv.h
@@ -124,12 +124,12 @@ struct arl {
 	int fd;
 	int wd_dir;
 	int wd_file;
-	char *filename;
+	const char *filename;
 };
 
 void arl_init(arl_t*);
 void arl_cleanup(arl_t*);
-void arl_setup(arl_t*, const char* /* result of realpath(3) */);
+void arl_add(arl_t*, const char* /* result of realpath(3) */);
 bool arl_handle(arl_t*);