From edb117e3bdb4d6bef4a4749d94144df8472c0a4d Mon Sep 17 00:00:00 2001 From: Max Voit Date: Thu, 26 Jan 2017 22:18:32 +0100 Subject: Add autoreload support by inotify (and dummy backend nop) --- Makefile | 7 +++ autoreload.h | 43 ++++++++++++++ autoreload_inotify.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++ autoreload_nop.c | 31 +++++++++++ main.c | 13 ++++- 5 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 autoreload.h create mode 100644 autoreload_inotify.c create mode 100644 autoreload_nop.c diff --git a/Makefile b/Makefile index f375fa3..4276326 100644 --- a/Makefile +++ b/Makefile @@ -24,6 +24,13 @@ endif .PHONY: clean install uninstall SRC := commands.c image.c main.c options.c thumbs.c util.c window.c +# conditionally compile in autoreload-backend; usage: `make AUTORELOAD=nop` +ifeq ($(AUTORELOAD),nop) + SRC += autoreload_nop.c +else + SRC += autoreload_inotify.c +endif + DEP := $(SRC:.c=.d) OBJ := $(SRC:.c=.o) diff --git a/autoreload.h b/autoreload.h new file mode 100644 index 0000000..439b695 --- /dev/null +++ b/autoreload.h @@ -0,0 +1,43 @@ +/* Copyright 2017 Max Voit + * + * This file is part of sxiv. + * + * sxiv is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * sxiv is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with sxiv. If not, see . + */ + +#ifndef AUTORELOAD_H +#define AUTORELOAD_H + +#include "types.h" + +void arl_cleanup(void); +void arl_handle(void); +void arl_init(void); +void arl_setup(void); +void arl_setup_dir(void); + +typedef struct { + int fd; + int wd; + bool watching_dir; +} autoreload_t; + +extern autoreload_t autoreload; +extern int fileidx; +extern fileinfo_t *files; + +void load_image(int); +void redraw(void); + +#endif /* AUTORELOAD_H */ diff --git a/autoreload_inotify.c b/autoreload_inotify.c new file mode 100644 index 0000000..4a8e455 --- /dev/null +++ b/autoreload_inotify.c @@ -0,0 +1,154 @@ +/* Copyright 2017 Max Voit + * + * This file is part of sxiv. + * + * sxiv is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * sxiv is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with sxiv. If not, see . + */ + +#include +#include +#include +#include +#include +#include + +#include "util.h" +#include "autoreload.h" + +const struct timespec ten_ms = {0, 10000000}; + +void arl_cleanup(void) +{ + if (autoreload.fd != -1 && autoreload.wd != -1) + { + if(inotify_rm_watch(autoreload.fd, autoreload.wd)) + error(0, 0, "Failed to remove inotify watch."); + } +} + +void arl_handle(void) +{ + ssize_t len; + char buf[4096] __attribute__ ((aligned(__alignof__(struct inotify_event)))); + const struct inotify_event *event; + char *ptr; + char *fntmp, *fn; + + len = read(autoreload.fd, buf, sizeof buf); + if (len == -1) + { + error(0, 0, "Failed to read inotify events."); + return; + } + + for (ptr = buf; ptr < buf + len; + ptr += sizeof(struct inotify_event) + event->len) + { + + event = (const struct inotify_event *) ptr; + + /* events from watching the file itself */ + if (event->mask & IN_CLOSE_WRITE) + { + load_image(fileidx); + redraw(); + } + + if (event->mask & IN_DELETE_SELF) + arl_setup_dir(); + + /* events from watching the file's directory */ + if (event->mask & IN_CREATE) + { + fntmp = strdup(files[fileidx].path); + fn = basename(fntmp); + + if (0 == strcmp(event->name, fn)) + { + /* this is the file we're looking for */ + + /* cleanup, this has not been one-shot */ + if (autoreload.watching_dir) + { + if(inotify_rm_watch(autoreload.fd, autoreload.wd)) + error(0, 0, "Failed to remove inotify watch."); + autoreload.watching_dir = false; + } + + /* when too fast, imlib2 can't load the image */ + nanosleep(&ten_ms, NULL); + load_image(fileidx); + redraw(); + } + free(fntmp); + } + } +} + +void arl_init(void) +{ + /* this needs to be done only once */ + autoreload.fd = inotify_init(); + autoreload.watching_dir = false; + if (autoreload.fd == -1) + error(0, 0, "Could not initialize inotify."); +} + +void arl_setup(void) +{ + if (autoreload.fd == -1) + { + error(0, 0, "Uninitialized, could not add inotify watch."); + return; + } + + /* may have switched from a deleted to another image */ + if (autoreload.watching_dir) + { + if (inotify_rm_watch(autoreload.fd, autoreload.wd)) + error(0, 0, "Failed to remove inotify watch."); + autoreload.watching_dir = false; + } + + autoreload.wd = inotify_add_watch(autoreload.fd, files[fileidx].path, + IN_ONESHOT | IN_CLOSE_WRITE | IN_DELETE_SELF); + if (autoreload.wd == -1) + error(0, 0, "Failed to add inotify watch on file '%s'.", files[fileidx].path); +} + +void arl_setup_dir(void) +{ + char *dntmp, *dn; + + if (autoreload.fd == -1) + { + error(0, 0, "Uninitialized, could not add inotify watch on directory."); + return; + } + + /* get dirname */ + dntmp = (char*) strdup(files[fileidx].path); + dn = (char*) dirname(dntmp); + + /* this is not one-shot as other stuff may be created too + note: we won't handle deletion of the directory itself, + this is a design decision */ + autoreload.wd = inotify_add_watch(autoreload.fd, dn,IN_CREATE); + if (autoreload.wd == -1) + error(0, 0, "Failed to add inotify watch on directory '%s'.", dn); + else + autoreload.watching_dir = true; + + free(dntmp); +} diff --git a/autoreload_nop.c b/autoreload_nop.c new file mode 100644 index 0000000..19641f8 --- /dev/null +++ b/autoreload_nop.c @@ -0,0 +1,31 @@ +/* Copyright 2017 Max Voit + * + * This file is part of sxiv. + * + * sxiv is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * sxiv is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with sxiv. If not, see . + */ + +#include "autoreload.h" + +void arl_cleanup(void) { } + +void arl_handle(void) { } + +void arl_init(void) { } + +void arl_setup(void) { } + +void arl_setup_dir(void) { } + + diff --git a/main.c b/main.c index 9e337f4..6214b94 100644 --- a/main.c +++ b/main.c @@ -38,6 +38,7 @@ #include "thumbs.h" #include "util.h" #include "window.h" +#include "autoreload.h" #define _MAPPINGS_CONFIG #include "config.h" @@ -64,6 +65,7 @@ fileinfo_t *files; int filecnt, fileidx; int alternate; int markcnt; +autoreload_t autoreload; int prefix; bool extprefix; @@ -98,6 +100,7 @@ timeout_t timeouts[] = { void cleanup(void) { img_close(&img, false); + arl_cleanup(); tns_free(&tns); win_close(&win); } @@ -317,6 +320,7 @@ void load_image(int new) info.open = false; open_info(); + arl_setup(); if (img.multi.cnt > 0 && img.multi.animate) set_timeout(animate, img.multi.frames[img.multi.sel].delay, true); @@ -685,7 +689,7 @@ void run(void) init_thumb = mode == MODE_THUMB && tns.initnext < filecnt; load_thumb = mode == MODE_THUMB && tns.loadnext < tns.end; - if ((init_thumb || load_thumb || to_set || info.fd != -1) && + if ((init_thumb || load_thumb || to_set || info.fd != -1 || autoreload.fd != -1) && XPending(win.env.dpy) == 0) { if (load_thumb) { @@ -708,9 +712,15 @@ void run(void) FD_SET(info.fd, &fds); xfd = MAX(xfd, info.fd); } + if (autoreload.fd != -1) { + FD_SET(autoreload.fd, &fds); + xfd = MAX(xfd, autoreload.fd); + } select(xfd + 1, &fds, 0, 0, to_set ? &timeout : NULL); if (info.fd != -1 && FD_ISSET(info.fd, &fds)) read_info(); + if (autoreload.fd != -1 && FD_ISSET(autoreload.fd, &fds)) + arl_handle(); } continue; } @@ -854,6 +864,7 @@ int main(int argc, char **argv) win_init(&win); img_init(&img, &win); + arl_init(); if ((homedir = getenv("XDG_CONFIG_HOME")) == NULL || homedir[0] == '\0') { homedir = getenv("HOME"); -- cgit v1.2.3 From 3724d3fc17dc6135a05608cab5bdf00c6978282d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:07:32 +0200 Subject: Revised autoreload interface Make the header only contain the public interface and nothing from the implementation. All functions get a handle to their self object, like the img_ and tns_ and win_ functions. All necessary data (file path) is also passed as an argument, so that no extern redeclarations are needed. Make arl_setup_dir() private, it's not called outside the module. Make arl_handle() return true if the file has changed, so that the reloading of the file can be done by the caller. --- autoreload.h | 18 +++------ autoreload_inotify.c | 110 +++++++++++++++++++++++++-------------------------- autoreload_nop.c | 32 ++++++++++----- main.c | 31 +++++++++------ 4 files changed, 100 insertions(+), 91 deletions(-) diff --git a/autoreload.h b/autoreload.h index 439b695..6a3053c 100644 --- a/autoreload.h +++ b/autoreload.h @@ -21,23 +21,15 @@ #include "types.h" -void arl_cleanup(void); -void arl_handle(void); -void arl_init(void); -void arl_setup(void); -void arl_setup_dir(void); - typedef struct { int fd; int wd; bool watching_dir; -} autoreload_t; - -extern autoreload_t autoreload; -extern int fileidx; -extern fileinfo_t *files; +} arl_t; -void load_image(int); -void redraw(void); +void arl_cleanup(arl_t*); +bool arl_handle(arl_t*, const char*); +void arl_init(arl_t*); +void arl_setup(arl_t*, const char*); #endif /* AUTORELOAD_H */ diff --git a/autoreload_inotify.c b/autoreload_inotify.c index 4a8e455..383e42b 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -21,35 +21,59 @@ #include #include #include -#include #include "util.h" #include "autoreload.h" -const struct timespec ten_ms = {0, 10000000}; - -void arl_cleanup(void) +CLEANUP void arl_cleanup(arl_t *arl) { - if (autoreload.fd != -1 && autoreload.wd != -1) + if (arl->fd != -1 && arl->wd != -1) { - if(inotify_rm_watch(autoreload.fd, autoreload.wd)) + if(inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); } } -void arl_handle(void) +static void arl_setup_dir(arl_t *arl, const char *filepath) +{ + char *dntmp, *dn; + + if (arl->fd == -1) + { + error(0, 0, "Uninitialized, could not add inotify watch on directory."); + return; + } + + /* get dirname */ + dntmp = (char*) strdup(filepath); + dn = (char*) dirname(dntmp); + + /* this is not one-shot as other stuff may be created too + note: we won't handle deletion of the directory itself, + this is a design decision */ + arl->wd = inotify_add_watch(arl->fd, dn,IN_CREATE); + if (arl->wd == -1) + error(0, 0, "Failed to add inotify watch on directory '%s'.", dn); + else + arl->watching_dir = true; + + free(dntmp); +} + +bool arl_handle(arl_t *arl, const char *filepath) { + bool reload = false; ssize_t len; char buf[4096] __attribute__ ((aligned(__alignof__(struct inotify_event)))); const struct inotify_event *event; char *ptr; char *fntmp, *fn; - len = read(autoreload.fd, buf, sizeof buf); + len = read(arl->fd, buf, sizeof buf); if (len == -1) { error(0, 0, "Failed to read inotify events."); - return; + return false; } for (ptr = buf; ptr < buf + len; @@ -61,17 +85,16 @@ void arl_handle(void) /* events from watching the file itself */ if (event->mask & IN_CLOSE_WRITE) { - load_image(fileidx); - redraw(); + reload = true; } if (event->mask & IN_DELETE_SELF) - arl_setup_dir(); + arl_setup_dir(arl, filepath); /* events from watching the file's directory */ if (event->mask & IN_CREATE) { - fntmp = strdup(files[fileidx].path); + fntmp = strdup(filepath); fn = basename(fntmp); if (0 == strcmp(event->name, fn)) @@ -79,76 +102,49 @@ void arl_handle(void) /* this is the file we're looking for */ /* cleanup, this has not been one-shot */ - if (autoreload.watching_dir) + if (arl->watching_dir) { - if(inotify_rm_watch(autoreload.fd, autoreload.wd)) + if(inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); - autoreload.watching_dir = false; + arl->watching_dir = false; } - /* when too fast, imlib2 can't load the image */ - nanosleep(&ten_ms, NULL); - load_image(fileidx); - redraw(); + reload = true; } free(fntmp); } } + return reload; } -void arl_init(void) +void arl_init(arl_t *arl) { /* this needs to be done only once */ - autoreload.fd = inotify_init(); - autoreload.watching_dir = false; - if (autoreload.fd == -1) + arl->fd = inotify_init(); + arl->watching_dir = false; + if (arl->fd == -1) error(0, 0, "Could not initialize inotify."); } -void arl_setup(void) +void arl_setup(arl_t *arl, const char *filepath) { - if (autoreload.fd == -1) + if (arl->fd == -1) { error(0, 0, "Uninitialized, could not add inotify watch."); return; } /* may have switched from a deleted to another image */ - if (autoreload.watching_dir) + if (arl->watching_dir) { - if (inotify_rm_watch(autoreload.fd, autoreload.wd)) + if (inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); - autoreload.watching_dir = false; + arl->watching_dir = false; } - autoreload.wd = inotify_add_watch(autoreload.fd, files[fileidx].path, + arl->wd = inotify_add_watch(arl->fd, filepath, IN_ONESHOT | IN_CLOSE_WRITE | IN_DELETE_SELF); - if (autoreload.wd == -1) - error(0, 0, "Failed to add inotify watch on file '%s'.", files[fileidx].path); + if (arl->wd == -1) + error(0, 0, "Failed to add inotify watch on file '%s'.", filepath); } -void arl_setup_dir(void) -{ - char *dntmp, *dn; - - if (autoreload.fd == -1) - { - error(0, 0, "Uninitialized, could not add inotify watch on directory."); - return; - } - - /* get dirname */ - dntmp = (char*) strdup(files[fileidx].path); - dn = (char*) dirname(dntmp); - - /* this is not one-shot as other stuff may be created too - note: we won't handle deletion of the directory itself, - this is a design decision */ - autoreload.wd = inotify_add_watch(autoreload.fd, dn,IN_CREATE); - if (autoreload.wd == -1) - error(0, 0, "Failed to add inotify watch on directory '%s'.", dn); - else - autoreload.watching_dir = true; - - free(dntmp); -} diff --git a/autoreload_nop.c b/autoreload_nop.c index 19641f8..41dbf47 100644 --- a/autoreload_nop.c +++ b/autoreload_nop.c @@ -18,14 +18,26 @@ #include "autoreload.h" -void arl_cleanup(void) { } - -void arl_handle(void) { } - -void arl_init(void) { } - -void arl_setup(void) { } - -void arl_setup_dir(void) { } - +void arl_cleanup(arl_t *arl) +{ + (void) arl; +} + +bool arl_handle(arl_t *arl, const char *filepath) +{ + (void) arl; + (void) filepath; + return false; +} + +void arl_init(arl_t *arl) +{ + (void) arl; +} + +void arl_setup(arl_t *arl, const char *filepath) +{ + (void) arl; + (void) filepath; +} diff --git a/main.c b/main.c index 6214b94..70f7521 100644 --- a/main.c +++ b/main.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -57,6 +58,7 @@ void slideshow(void); void clear_resize(void); appmode_t mode; +arl_t arl; img_t img; tns_t tns; win_t win; @@ -65,7 +67,6 @@ fileinfo_t *files; int filecnt, fileidx; int alternate; int markcnt; -autoreload_t autoreload; int prefix; bool extprefix; @@ -100,7 +101,7 @@ timeout_t timeouts[] = { void cleanup(void) { img_close(&img, false); - arl_cleanup(); + arl_cleanup(&arl); tns_free(&tns); win_close(&win); } @@ -320,7 +321,7 @@ void load_image(int new) info.open = false; open_info(); - arl_setup(); + arl_setup(&arl, files[fileidx].path); if (img.multi.cnt > 0 && img.multi.animate) set_timeout(animate, img.multi.frames[img.multi.sel].delay, true); @@ -676,6 +677,8 @@ void on_buttonpress(XButtonEvent *bev) prefix = 0; } +const struct timespec ten_ms = {0, 10000000}; + void run(void) { int xfd; @@ -689,8 +692,8 @@ void run(void) init_thumb = mode == MODE_THUMB && tns.initnext < filecnt; load_thumb = mode == MODE_THUMB && tns.loadnext < tns.end; - if ((init_thumb || load_thumb || to_set || info.fd != -1 || autoreload.fd != -1) && - XPending(win.env.dpy) == 0) + if ((init_thumb || load_thumb || to_set || info.fd != -1 || + arl.fd != -1) && XPending(win.env.dpy) == 0) { if (load_thumb) { set_timeout(redraw, TO_REDRAW_THUMBS, false); @@ -712,15 +715,21 @@ void run(void) FD_SET(info.fd, &fds); xfd = MAX(xfd, info.fd); } - if (autoreload.fd != -1) { - FD_SET(autoreload.fd, &fds); - xfd = MAX(xfd, autoreload.fd); + if (arl.fd != -1) { + FD_SET(arl.fd, &fds); + xfd = MAX(xfd, arl.fd); } select(xfd + 1, &fds, 0, 0, to_set ? &timeout : NULL); if (info.fd != -1 && FD_ISSET(info.fd, &fds)) read_info(); - if (autoreload.fd != -1 && FD_ISSET(autoreload.fd, &fds)) - arl_handle(); + if (arl.fd != -1 && FD_ISSET(arl.fd, &fds)) { + if (arl_handle(&arl, files[fileidx].path)) { + /* when too fast, imlib2 can't load the image */ + nanosleep(&ten_ms, NULL); + load_image(fileidx); + redraw(); + } + } } continue; } @@ -864,7 +873,7 @@ int main(int argc, char **argv) win_init(&win); img_init(&img, &win); - arl_init(); + arl_init(&arl); if ((homedir = getenv("XDG_CONFIG_HOME")) == NULL || homedir[0] == '\0') { homedir = getenv("HOME"); -- cgit v1.2.3 From 8aaa5c9398b6cbb0a85ed77812c3f6d11fe80f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:11:44 +0200 Subject: Simplify autoreload backend selection in Makefile --- Makefile | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 4276326..8b3f4b5 100644 --- a/Makefile +++ b/Makefile @@ -21,16 +21,13 @@ ifndef NO_LIBEXIF LIBS += -lexif endif -.PHONY: clean install uninstall +# select autoreload backend +# overwritten with `make AUTORELOAD=nop` +AUTORELOAD := inotify -SRC := commands.c image.c main.c options.c thumbs.c util.c window.c -# conditionally compile in autoreload-backend; usage: `make AUTORELOAD=nop` -ifeq ($(AUTORELOAD),nop) - SRC += autoreload_nop.c -else - SRC += autoreload_inotify.c -endif +.PHONY: clean install uninstall +SRC := autoreload_$(AUTORELOAD).c commands.c image.c main.c options.c thumbs.c util.c window.c DEP := $(SRC:.c=.d) OBJ := $(SRC:.c=.o) -- cgit v1.2.3 From 9ac8fc62dfc8061c6bc1f973454ba10228acfbf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:12:22 +0200 Subject: Fix code-style in autoreload_inotify.c --- autoreload_inotify.c | 61 ++++++++++++++++++++-------------------------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/autoreload_inotify.c b/autoreload_inotify.c index 383e42b..b536c2a 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -27,9 +27,8 @@ CLEANUP void arl_cleanup(arl_t *arl) { - if (arl->fd != -1 && arl->wd != -1) - { - if(inotify_rm_watch(arl->fd, arl->wd)) + if (arl->fd != -1 && arl->wd != -1) { + if (inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); } } @@ -38,8 +37,7 @@ static void arl_setup_dir(arl_t *arl, const char *filepath) { char *dntmp, *dn; - if (arl->fd == -1) - { + if (arl->fd == -1) { error(0, 0, "Uninitialized, could not add inotify watch on directory."); return; } @@ -49,9 +47,10 @@ static void arl_setup_dir(arl_t *arl, const char *filepath) dn = (char*) dirname(dntmp); /* this is not one-shot as other stuff may be created too - note: we won't handle deletion of the directory itself, - this is a design decision */ - arl->wd = inotify_add_watch(arl->fd, dn,IN_CREATE); + * note: we won't handle deletion of the directory itself, + * this is a design decision + */ + arl->wd = inotify_add_watch(arl->fd, dn, IN_CREATE); if (arl->wd == -1) error(0, 0, "Failed to add inotify watch on directory '%s'.", dn); else @@ -63,52 +62,40 @@ static void arl_setup_dir(arl_t *arl, const char *filepath) bool arl_handle(arl_t *arl, const char *filepath) { bool reload = false; - ssize_t len; char buf[4096] __attribute__ ((aligned(__alignof__(struct inotify_event)))); - const struct inotify_event *event; char *ptr; - char *fntmp, *fn; + const struct inotify_event *event; + + ssize_t len = read(arl->fd, buf, sizeof(buf)); - len = read(arl->fd, buf, sizeof buf); - if (len == -1) - { + if (len == -1) { error(0, 0, "Failed to read inotify events."); return false; } - - for (ptr = buf; ptr < buf + len; - ptr += sizeof(struct inotify_event) + event->len) - { - - event = (const struct inotify_event *) ptr; + for (ptr = buf; ptr < buf + len; ptr += sizeof(*event) + event->len) { + event = (const struct inotify_event*) ptr; /* events from watching the file itself */ - if (event->mask & IN_CLOSE_WRITE) - { + if (event->mask & IN_CLOSE_WRITE) { reload = true; } - if (event->mask & IN_DELETE_SELF) arl_setup_dir(arl, filepath); /* events from watching the file's directory */ - if (event->mask & IN_CREATE) - { - fntmp = strdup(filepath); - fn = basename(fntmp); + if (event->mask & IN_CREATE) { + char *fntmp = strdup(filepath); + char *fn = basename(fntmp); - if (0 == strcmp(event->name, fn)) - { + if (STREQ(event->name, fn)) { /* this is the file we're looking for */ /* cleanup, this has not been one-shot */ - if (arl->watching_dir) - { - if(inotify_rm_watch(arl->fd, arl->wd)) + if (arl->watching_dir) { + if (inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); arl->watching_dir = false; } - reload = true; } free(fntmp); @@ -128,22 +115,20 @@ void arl_init(arl_t *arl) void arl_setup(arl_t *arl, const char *filepath) { - if (arl->fd == -1) - { + if (arl->fd == -1) { error(0, 0, "Uninitialized, could not add inotify watch."); return; } /* may have switched from a deleted to another image */ - if (arl->watching_dir) - { + if (arl->watching_dir) { if (inotify_rm_watch(arl->fd, arl->wd)) error(0, 0, "Failed to remove inotify watch."); arl->watching_dir = false; } arl->wd = inotify_add_watch(arl->fd, filepath, - IN_ONESHOT | IN_CLOSE_WRITE | IN_DELETE_SELF); + IN_ONESHOT | IN_CLOSE_WRITE | IN_DELETE_SELF); if (arl->wd == -1) error(0, 0, "Failed to add inotify watch on file '%s'.", filepath); } -- cgit v1.2.3 From 8bce80fdae23d0747efc8547fd33edbd246b54b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:13:32 +0200 Subject: Revised error reporting in autoreload_inotify No repeated error messages after failed initialization. No error messages on failed inotify_rm_watch(). --- autoreload_inotify.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/autoreload_inotify.c b/autoreload_inotify.c index b536c2a..e44c2f3 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -27,20 +27,16 @@ CLEANUP void arl_cleanup(arl_t *arl) { - if (arl->fd != -1 && arl->wd != -1) { - if (inotify_rm_watch(arl->fd, arl->wd)) - error(0, 0, "Failed to remove inotify watch."); - } + if (arl->fd != -1 && arl->wd != -1) + inotify_rm_watch(arl->fd, arl->wd); } static void arl_setup_dir(arl_t *arl, const char *filepath) { char *dntmp, *dn; - if (arl->fd == -1) { - error(0, 0, "Uninitialized, could not add inotify watch on directory."); + if (arl->fd == -1) return; - } /* get dirname */ dntmp = (char*) strdup(filepath); @@ -52,7 +48,7 @@ static void arl_setup_dir(arl_t *arl, const char *filepath) */ arl->wd = inotify_add_watch(arl->fd, dn, IN_CREATE); if (arl->wd == -1) - error(0, 0, "Failed to add inotify watch on directory '%s'.", dn); + error(0, 0, "%s: Error watching directory", dn); else arl->watching_dir = true; @@ -68,10 +64,9 @@ bool arl_handle(arl_t *arl, const char *filepath) ssize_t len = read(arl->fd, buf, sizeof(buf)); - if (len == -1) { - error(0, 0, "Failed to read inotify events."); + if (len == -1) return false; - } + for (ptr = buf; ptr < buf + len; ptr += sizeof(*event) + event->len) { event = (const struct inotify_event*) ptr; @@ -92,8 +87,7 @@ bool arl_handle(arl_t *arl, const char *filepath) /* cleanup, this has not been one-shot */ if (arl->watching_dir) { - if (inotify_rm_watch(arl->fd, arl->wd)) - error(0, 0, "Failed to remove inotify watch."); + inotify_rm_watch(arl->fd, arl->wd); arl->watching_dir = false; } reload = true; @@ -110,26 +104,23 @@ void arl_init(arl_t *arl) arl->fd = inotify_init(); arl->watching_dir = false; if (arl->fd == -1) - error(0, 0, "Could not initialize inotify."); + error(0, 0, "Could not initialize inotify, no automatic image reloading"); } void arl_setup(arl_t *arl, const char *filepath) { - if (arl->fd == -1) { - error(0, 0, "Uninitialized, could not add inotify watch."); + if (arl->fd == -1) return; - } /* may have switched from a deleted to another image */ if (arl->watching_dir) { - if (inotify_rm_watch(arl->fd, arl->wd)) - error(0, 0, "Failed to remove inotify watch."); + 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, "Failed to add inotify watch on file '%s'.", filepath); + error(0, 0, "%s: Error watching file", filepath); } -- cgit v1.2.3 From 6695cd4c3409a5a0873cc041fbfa6bb38711149d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:14:20 +0200 Subject: Simplify inotify cleanup --- autoreload_inotify.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/autoreload_inotify.c b/autoreload_inotify.c index e44c2f3..8e4d67a 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -27,8 +27,8 @@ CLEANUP void arl_cleanup(arl_t *arl) { - if (arl->fd != -1 && arl->wd != -1) - inotify_rm_watch(arl->fd, arl->wd); + if (arl->fd != -1) + close(arl->fd); } static void arl_setup_dir(arl_t *arl, const char *filepath) -- cgit v1.2.3 From 0e1a85d22485e2aac1b9930d1e3e0897a2076db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:14:36 +0200 Subject: Read all available inotify events Loop reading from inotify fd in arl_handle(); requires non-blocking inotify fd. --- autoreload_inotify.c | 59 ++++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/autoreload_inotify.c b/autoreload_inotify.c index 8e4d67a..a7378f6 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -16,6 +16,7 @@ * along with sxiv. If not, see . */ +#include #include #include #include @@ -62,37 +63,41 @@ bool arl_handle(arl_t *arl, const char *filepath) char *ptr; const struct inotify_event *event; - ssize_t len = read(arl->fd, buf, sizeof(buf)); + for (;;) { + ssize_t len = read(arl->fd, buf, sizeof(buf)); - if (len == -1) - return false; - - for (ptr = buf; ptr < buf + 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; + if (len == -1) { + if (errno == EINTR) + continue; + break; } - if (event->mask & IN_DELETE_SELF) - arl_setup_dir(arl, filepath); - - /* 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 */ + for (ptr = buf; ptr < buf + len; ptr += sizeof(*event) + event->len) { + event = (const struct inotify_event*) ptr; - /* cleanup, this has not been one-shot */ - if (arl->watching_dir) { - inotify_rm_watch(arl->fd, arl->wd); - arl->watching_dir = false; - } + /* events from watching the file itself */ + if (event->mask & IN_CLOSE_WRITE) { reload = true; } - free(fntmp); + if (event->mask & IN_DELETE_SELF) + arl_setup_dir(arl, filepath); + + /* 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; @@ -101,7 +106,7 @@ bool arl_handle(arl_t *arl, const char *filepath) void arl_init(arl_t *arl) { /* this needs to be done only once */ - arl->fd = inotify_init(); + 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"); -- cgit v1.2.3 From de3d7827ce13a52b1bcf3b72e9dc2ee89152dea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:15:35 +0200 Subject: Compiler independent buffer alignment --- autoreload_inotify.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/autoreload_inotify.c b/autoreload_inotify.c index a7378f6..473b8cb 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -56,22 +56,26 @@ static void arl_setup_dir(arl_t *arl, const char *filepath) free(dntmp); } +union { + char d[4096]; /* aligned buffer */ + struct inotify_event e; +} buf; + bool arl_handle(arl_t *arl, const char *filepath) { bool reload = false; - char buf[4096] __attribute__ ((aligned(__alignof__(struct inotify_event)))); char *ptr; const struct inotify_event *event; for (;;) { - ssize_t len = read(arl->fd, buf, sizeof(buf)); + ssize_t len = read(arl->fd, buf.d, sizeof(buf.d)); if (len == -1) { if (errno == EINTR) continue; break; } - for (ptr = buf; ptr < buf + len; ptr += sizeof(*event) + event->len) { + 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 */ -- cgit v1.2.3 From a20173a42df64515c0a5d1c5fba0c056a633a441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 17 May 2017 20:16:16 +0200 Subject: 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 | 11 +++--- autoreload_inotify.c | 110 +++++++++++++++++++++------------------------------ main.c | 2 +- 3 files changed, 51 insertions(+), 72 deletions(-) diff --git a/autoreload.h b/autoreload.h index 6a3053c..bb30eb6 100644 --- a/autoreload.h +++ b/autoreload.h @@ -23,13 +23,14 @@ typedef struct { int fd; - int wd; - bool watching_dir; + int wd_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 */ diff --git a/autoreload_inotify.c b/autoreload_inotify.c index 473b8cb..fe05483 100644 --- a/autoreload_inotify.c +++ b/autoreload_inotify.c @@ -21,39 +21,60 @@ #include #include #include -#include #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 */ - dntmp = (char*) strdup(filepath); - dn = (char*) dirname(dntmp); - - /* this is not one-shot as other stuff may be created too - * note: we won't handle deletion of the directory itself, - * this is a design decision - */ - arl->wd = inotify_add_watch(arl->fd, dn, IN_CREATE); - if (arl->wd == -1) - error(0, 0, "%s: Error watching directory", dn); - else - arl->watching_dir = true; - - free(dntmp); + 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); + } } 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; - } - if (event->mask & IN_DELETE_SELF) - arl_setup_dir(arl, filepath); - - /* 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; - } + } else if (event->mask & IN_DELETE_SELF) { + rm_watch(arl->fd, &arl->wd_file); + } else if (event->mask & (IN_CREATE | IN_MOVED_TO)) { + if (STREQ(event->name, arl->filename)) 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); -} - diff --git a/main.c b/main.c index 70f7521..aa11616 100644 --- a/main.c +++ b/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); -- cgit v1.2.3