watcher: refactor

- Use negated errno as return value.
- Do not consider deletion of the watched file as a fatal error,
  and use a separate error code to distinguish between them.
- Lazy-init worker: Starts watching upon the first call to
  watcher_poll().

src/backend_chromium.c

@@ -1612,7 +1612,7 @@ store_load (
     struct watcher *watcher = ctx->watcher;
     if (unlikely(watcher == NULL)) {
         if (0 != init_watcher(ctx)) {
-            return -1;
+            return -EIO;
         }
         goto do_load;
     }
@@ -1626,22 +1626,22 @@ store_load (
 #endif  /* defined(BOOKMARKFS_BACKEND_CHROMIUM_WRITE) */
 
     switch (watcher_poll(watcher)) {
-      case WATCHER_POLL_ERR:
+      case -EAGAIN:
-        return -1;
-
-      case WATCHER_POLL_NOCHANGE:
         if (ctx->store == NULL) {
             break;
         }
         return 0;
 
-      case WATCHER_POLL_CHANGED:
+      case 0:
         json_decref(ctx->store);
         ctx->store = NULL;
         break;
 
+      case -ENOENT:
+        return -ENXIO;
+
       default:
-        unreachable();
+        return -EIO;
     }
 
   do_load:
@@ -1878,11 +1878,11 @@ bookmark_check (
     if (ctx->flags & BACKEND_FILENAME_GUID) {
         return -EPERM;
     }
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
-    int status = 0;
     struct bookmark_lcookie *cookie;
     size_t idx = 0;
     if (cookie_ptr != NULL) {
@@ -1940,8 +1940,9 @@ bookmark_get (
 ) {
     struct backend_ctx *ctx = backend_ctx;
 
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     if (cookie_ptr == NULL) {
@@ -1972,8 +1973,7 @@ bookmark_get (
     }
 
     json_t *value_node;
-    int status = get_xattr_val(entry->node, xattr_name, ctx->flags,
+    status = get_xattr_val(entry->node, xattr_name, ctx->flags, &value_node);
-            &value_node);
     if (status < 0) {
         return status;
     }
@@ -2016,11 +2016,11 @@ bookmark_list (
 ) {
     struct backend_ctx *ctx = backend_ctx;
 
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
-    int status = 0;
     struct bookmark_lcookie *cookie;
     struct node_entry const *entry = NULL;
     json_t *children;
@@ -2097,8 +2097,9 @@ bookmark_lookup (
 ) {
     struct backend_ctx *ctx = backend_ctx;
 
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     struct node_entry *entry = lookup_id(ctx->id_map, id, NULL, NULL);
@@ -2202,8 +2203,9 @@ bookmark_create (
     if (parent_id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     // Lookup parent entry
@@ -2245,7 +2247,7 @@ bookmark_create (
     struct build_node_ctx bctx = {
         .stat_buf = stat_buf,
     };
-    int status = build_node(ctx, node, &name, name_len, is_dir, &bctx);
+    status = build_node(ctx, node, &name, name_len, is_dir, &bctx);
     if (unlikely(status != 0)) {
         json_decref(node);
         return status;
@@ -2306,8 +2308,9 @@ bookmark_delete (
     if (parent_id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     // Lookup parent entry
@@ -2373,8 +2376,9 @@ bookmark_permute (
     if (parent_id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     size_t name1_len = strnlen(name1, NAME_MAX + 1);
@@ -2464,8 +2468,9 @@ bookmark_rename (
             || new_parent_id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
-    if (0 != store_load(ctx)) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     // Lookup old entry
@@ -2612,8 +2617,9 @@ bookmark_set (
     if (id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
-    if (unlikely(0 != store_load(ctx))) {
+    int status = store_load(ctx);
-        return -EIO;
+    if (unlikely(status < 0)) {
+        return status;
     }
 
     unsigned long entry_id;

src/watcher.c

@@ -111,8 +111,8 @@ impl_init (
         .flags  = EV_ADD,
     };
     if (0 != kevent(kqfd, &ev, 1, NULL, 0, NULL)) {
-        close(kqfd);
         log_printf("kevent(): %s", xstrerror(errno));
+        close(kqfd);
         return -1;
     }
     w->kqfd = kqfd;
@@ -146,15 +146,20 @@ static int
 impl_rearm (
     struct watcher *w
 ) {
+    if (unlikely(w->flags & WATCHER_DEAD)) {
+        log_puts("worker is dead");
+        return -EIO;
+    }
     if (w->flags & WATCHER_FALLBACK) {
         goto fallback;
     }
+    int err;
 
 #if defined(WATCHER_IMPL_FANOTIFY)
     uint32_t mask = FAN_DELETE_SELF | FAN_MOVE_SELF | FAN_MODIFY;
     if (0 != fanotify_mark(w->fanfd, FAN_MARK_ADD, mask, w->dirfd, w->name)) {
-        log_printf("fanotify_mark(): %s", xstrerror(errno));
+        log_printf("fanotify_mark(): %s", xstrerror_save(&err));
-        return -1;
+        goto fail;
     }
 
 #elif defined(WATCHER_IMPL_KQUEUE)
@@ -164,8 +169,8 @@ impl_rearm (
 #endif
     int wfd = openat(w->dirfd, w->name, open_flags);
     if (wfd < 0) {
-        log_printf("openat(): %s", xstrerror(errno));
+        log_printf("openat(): %s", xstrerror_save(&err));
-        return -1;
+        goto fail;
     }
     struct kevent ev = {
         .ident  = wfd,
@@ -174,9 +179,9 @@ impl_rearm (
         .fflags = NOTE_DELETE | NOTE_EXTEND | NOTE_RENAME | NOTE_WRITE,
     };
     if (0 != kevent(w->kqfd, &ev, 1, NULL, 0, NULL)) {
-        close(wfd);
         log_printf("kevent(): %s", xstrerror(errno));
-        return -1;
+        close(wfd);
+        return -EIO;
     }
     w->wfd = wfd;
 
@@ -185,10 +190,13 @@ impl_rearm (
 
   fallback:
     if (0 != fstatat(w->dirfd, w->name, &w->old_stat, 0)) {
-        log_printf("fstatat(): %s", xstrerror(errno));
+        log_printf("fstatat(): %s", xstrerror_save(&err));
-        return -1;
+        goto fail;
     }
     return 0;
+
+  fail:
+    return err == ENOENT ? -ENOENT : -EIO;
 }
 
 static int
@@ -276,7 +284,10 @@ impl_watch (
 
         struct stat new_stat;
         if (0 != fstatat(w->dirfd, w->name, &new_stat, 0)) {
-            debug_printf("fstatat(): %s", xstrerror(errno));
+            if (errno != ENOENT) {
+                debug_printf("fstatat(): %s", xstrerror(errno));
+                return -1;
+            }
             break;
         }
         if (new_stat.st_ino != old_stat->st_ino
@@ -311,14 +322,11 @@ worker_loop (
     }
 #endif  /* defined(BOOKMARKFS_SANDBOX) */
 
-    if (0 != impl_rearm(w)) {
-        goto end;
-    }
     debug_puts("worker ready");
-    while (0 == impl_watch(w)) {
+    do {
         w->flags |= WATCHER_IDLE;
         pthread_cond_wait(&w->cond, &w->mutex);
-    }
+    } while (0 == impl_watch(w));
 
   end:
     w->flags |= (WATCHER_DEAD | WATCHER_IDLE);
@@ -409,7 +417,7 @@ int
 watcher_poll (
     struct watcher *w
 ) {
-    int status = WATCHER_POLL_NOCHANGE;
+    int status = -EAGAIN;
     if (w->pipefd[1] < 0) {
         // WATCHER_NOOP
         return status;
@@ -424,16 +432,10 @@ watcher_poll (
         goto end;
     }
 
-    status = WATCHER_POLL_ERR;
+    status = impl_rearm(w);
-    if (unlikely(w->flags & WATCHER_DEAD)) {
+    if (status < 0) {
-        log_puts("worker is dead");
         goto end;
     }
-    if (0 != impl_rearm(w)) {
-        goto end;
-    }
-
-    status = WATCHER_POLL_CHANGED;
     w->flags &= ~WATCHER_IDLE;
     pthread_cond_signal(&w->cond);
 

src/watcher.h

@@ -36,18 +36,7 @@
 #endif
 
 /**
- * A watcher_poll() call returning WATCHER_POLL_ERR
+ * Always use fstatat() to detect file change.
- * most likely means that the file being watched has gone.
- *
- * When the file is back, calling watcher_poll() again should
- * return WATCHER_POLL_CHANGED, and the watcher should continue to work.
- */
-#define WATCHER_POLL_ERR       -1
-#define WATCHER_POLL_NOCHANGE   0
-#define WATCHER_POLL_CHANGED    1
-
-/**
- * Always use fstat() to detect file change.
  */
 #define WATCHER_FALLBACK  ( 1u << 0 )
 /**
@@ -84,10 +73,13 @@ watcher_destroy (
 
 /**
  * Check whether the file associated with the watcher
- * has changed since the last watcher_poll() call
+ * has changed since the last watcher_poll() call.
- * (or, if not yet called, since watcher initialization).
  *
- * Returns one of WATCHER_POLL_*.
+ * - Returns 0 if the file has changed, -EAGAIN if not.
+ * - Returns -ENOENT if the file being watched has gone.
+ * - Returns -EIO if an internal error occurred,
+ *   in which case the watcher became defunct,
+ *   and further calls to this function always fail.
  */
 int
 watcher_poll (