Detect all file overwrites in autoreload_inotify

mv(1) inside the same filesystem was not detected.

Supporting this case made it necessary to always watch the directory. Turns out
the logic and state keeping between arl_setup() and arl_handle() is easier,
when using different watch descriptors for the file and the directory and not
using a oneshot descriptor for the file.

Requiring an absolute canonical path for arl_setup() simplifies dir and base
name splitting. No need for dirname(3) and basename(3) anymore.

autoreload.h

@@ -23,13 +23,14 @@
 
 typedef struct {
 	int fd;
-	int wd;
+	int wd_dir;
-	bool watching_dir;
+	int wd_file;
+	char *filename;
 } arl_t;
 
-void arl_cleanup(arl_t*);
-bool arl_handle(arl_t*, const char*);
 void arl_init(arl_t*);
-void arl_setup(arl_t*, const char*);
+void arl_cleanup(arl_t*);
+void arl_setup(arl_t*, const char* /* result of realpath(3) */);
+bool arl_handle(arl_t*);
 
 #endif /* AUTORELOAD_H */

autoreload_inotify.c

@@ -21,39 +21,60 @@
 #include <string.h>
 #include <unistd.h>
 #include <sys/inotify.h>
-#include <libgen.h>
 
 #include "util.h"
 #include "autoreload.h"
 
+void arl_init(arl_t *arl)
+{
+	arl->fd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK);
+	arl->wd_dir = arl->wd_file = -1;
+	if (arl->fd == -1)
+		error(0, 0, "Could not initialize inotify, no automatic image reloading");
+}
+
 CLEANUP void arl_cleanup(arl_t *arl)
 {
 	if (arl->fd != -1)
 		close(arl->fd);
+	free(arl->filename);
 }
 
-static void arl_setup_dir(arl_t *arl, const char *filepath)
+static void rm_watch(int fd, int *wd)
 {
-	char *dntmp, *dn;
+	if (*wd != -1) {
+		inotify_rm_watch(fd, *wd);
+		*wd = -1;
+	}
+}
+
+static void add_watch(int fd, int *wd, const char *path, uint32_t mask)
+{
+	*wd = inotify_add_watch(fd, path, mask);
+	if (*wd == -1)
+		error(0, errno, "inotify: %s", path);
+}
+
+void arl_setup(arl_t *arl, const char *filepath)
+{
+	char *base = strrchr(filepath, '/');
 
 	if (arl->fd == -1)
 		return;
 
-	/* get dirname */
+	rm_watch(arl->fd, &arl->wd_dir);
-	dntmp = (char*) strdup(filepath);
+	rm_watch(arl->fd, &arl->wd_file);
-	dn = (char*) dirname(dntmp);
+	add_watch(arl->fd, &arl->wd_file, filepath, IN_CLOSE_WRITE | IN_DELETE_SELF);
-
+	free(arl->filename);
-	/* this is not one-shot as other stuff may be created too
+	arl->filename = estrdup(filepath);
-	 * note: we won't handle deletion of the directory itself,
+	if (base != NULL) {
-	 * this is a design decision
+		arl->filename[++base - filepath] = '\0';
-	 */
+		add_watch(arl->fd, &arl->wd_dir, arl->filename, IN_CREATE | IN_MOVED_TO);
-	arl->wd = inotify_add_watch(arl->fd, dn, IN_CREATE);
+		strcpy(arl->filename, base);
-	if (arl->wd == -1)
+	}
-		error(0, 0, "%s: Error watching directory", dn);
-	else
-		arl->watching_dir = true;
-
-	free(dntmp);
 }
 
 union {
@@ -61,7 +82,7 @@ union {
 	struct inotify_event e;
 } buf;
 
-bool arl_handle(arl_t *arl, const char *filepath)
+bool arl_handle(arl_t *arl)
 {
 	bool reload = false;
 	char *ptr;
@@ -77,59 +98,16 @@ bool arl_handle(arl_t *arl, const char *filepath)
 		}
 		for (ptr = buf.d; ptr < buf.d + len; ptr += sizeof(*event) + event->len) {
 			event = (const struct inotify_event*) ptr;
-
-			/* events from watching the file itself */
 			if (event->mask & IN_CLOSE_WRITE) {
 				reload = true;
-			}
+			} else if (event->mask & IN_DELETE_SELF) {
-			if (event->mask & IN_DELETE_SELF)
+				rm_watch(arl->fd, &arl->wd_file);
-				arl_setup_dir(arl, filepath);
+			} else if (event->mask & (IN_CREATE | IN_MOVED_TO)) {
-
+				if (STREQ(event->name, arl->filename))
-			/* events from watching the file's directory */
-			if (event->mask & IN_CREATE) {
-				char *fntmp = strdup(filepath);
-				char *fn = basename(fntmp);
-
-				if (STREQ(event->name, fn)) {
-					/* this is the file we're looking for */
-
-					/* cleanup, this has not been one-shot */
-					if (arl->watching_dir) {
-						inotify_rm_watch(arl->fd, arl->wd);
-						arl->watching_dir = false;
-					}
 					reload = true;
-				}
-				free(fntmp);
 			}
 		}
 	}
 	return reload;
 }
 
-void arl_init(arl_t *arl)
-{
-	/* this needs to be done only once */
-	arl->fd = inotify_init1(IN_CLOEXEC | IN_NONBLOCK);
-	arl->watching_dir = false;
-	if (arl->fd == -1)
-		error(0, 0, "Could not initialize inotify, no automatic image reloading");
-}
-
-void arl_setup(arl_t *arl, const char *filepath)
-{
-	if (arl->fd == -1)
-		return;
-
-	/* may have switched from a deleted to another image */
-	if (arl->watching_dir) {
-		inotify_rm_watch(arl->fd, arl->wd);
-		arl->watching_dir = false;
-	}
-
-	arl->wd = inotify_add_watch(arl->fd, filepath,
-		                          IN_ONESHOT | IN_CLOSE_WRITE | IN_DELETE_SELF);
-	if (arl->wd == -1)
-		error(0, 0, "%s: Error watching file", filepath);
-}
-

main.c

@@ -723,7 +723,7 @@ void run(void)
 				if (info.fd != -1 && FD_ISSET(info.fd, &fds))
 					read_info();
 				if (arl.fd != -1 && FD_ISSET(arl.fd, &fds)) {
-					if (arl_handle(&arl, files[fileidx].path)) {
+					if (arl_handle(&arl)) {
 						/* when too fast, imlib2 can't load the image */
 						nanosleep(&ten_ms, NULL);
 						load_image(fileidx);