backend: refactor bookmark_set() for timestamps

Do not expose UTIME_OMIT to backends, but instead specify
which timestamps to update with flags.

This allows us to further refactor backend code, especially
the Chromium backend.

doc/bookmarkfs.texi

@@ -2491,22 +2491,18 @@ value instead of the bookmark content.
 A bit array of the following flags:
 
 @table @code
-@item BOOKMARKFS_BOOKMARK_SET_TIME
+@item BOOKMARKFS_BOOKMARK_SET_ATIME
+@item BOOKMARKFS_BOOKMARK_SET_MTIME
 Indicates that the function should update the timestamps associated with
-the object.
+the object, instead of the bookmark content or extended attribute value.
 
 The @code{val} argument points to an array of @code{struct timespec},
 the first element refers to the last access time, and the second element
-refers to the last modification time of the object.
+refers to the last modification time for the object.
+If a flag is not set, the corresponding timestamp should be left as-is.
+
 Values of @code{xattr_name} and @code{val_len} are unspecified.
 
-The @code{tv_nsec} field of each timestamp may be @code{UTIME_OMIT},
-which indicates that the timestamp should be left as-is.
-It is guaranteed not to be @code{UTIME_NOW}.
-
-If this flag is not set, the function should update the bookmark content
-or extended attribute value instead.
-
 @item BOOKMARKFS_BOOKMARK_TYPE_MASK
 The subsystem to operate on.
 

src/backend.h

@@ -60,7 +60,8 @@
 #define BOOKMARKFS_BOOKMARK_CREATE_DIR        ( 1u << 0 )
 #define BOOKMARKFS_BOOKMARK_LIST_WITHSTAT     ( 1u << 0 )
 #define BOOKMARKFS_BOOKMARK_RENAME_NOREPLACE  ( 1u << 0 )
-#define BOOKMARKFS_BOOKMARK_SET_TIME          ( 1u << 0 )
+#define BOOKMARKFS_BOOKMARK_SET_ATIME         ( 1u << 0 )
+#define BOOKMARKFS_BOOKMARK_SET_MTIME         ( 1u << 1 )
 
 #define BOOKMARKFS_BOOKMARK_TYPE_BITS   3
 #define BOOKMARKFS_BOOKMARK_TYPE_SHIFT  ( 32 - BOOKMARKFS_BOOKMARK_TYPE_BITS )

src/backend_chromium.c

@@ -37,7 +37,6 @@
 
 #include <fcntl.h>
 #include <iconv.h>
-#include <sys/stat.h>
 #include <unistd.h>
 
 #ifdef BOOKMARKFS_BACKEND_CHROMIUM_WRITE
@@ -205,22 +204,20 @@ static int  fsck_apply      (struct backend_ctx *, uint64_t,
                              struct bookmarkfs_fsck_data const *,
                              bookmarkfs_bookmark_check_cb *, void *);
 static int  init_iconv      (iconv_t *);
-static int  node_mtime_now  (json_t *, json_t **);
+static void node_mtime_now  (json_t *, json_t **);
 static int  parse_mkfsopts  (struct bookmarkfs_conf_opt const *,
                              struct parsed_mkfsopts *);
-static int  store_new       (struct timespec *, json_t **);
+static int  store_new       (struct timespec const *, json_t **);
 static int  store_save      (struct backend_ctx *);
 static void update_guid     (struct node_entry *, struct hashmap *,
                              unsigned long, uint8_t *, unsigned long);
-static int  update_node_ts  (json_t *, struct timespec *);
 #endif  /* defined(BOOKMARKFS_BACKEND_CHROMIUM_WRITE) */
 
 static int  assocmap_comp (union hashmap_key, void const *);
 static unsigned long
             assocmap_hash (void const *);
 static int  build_maps    (struct backend_ctx *);
-static int  build_ts      (struct timespec *, char *, size_t);
+static void build_tsnode  (struct timespec const *, json_t **);
-static int  build_tsnode  (struct timespec *, json_t **);
 static void free_bgcookie (struct bookmark_gcookie *);
 static void free_blcookie (struct bookmark_lcookie *);
 static void free_entry_cb (void *, void *);
@@ -289,11 +286,10 @@ build_node (
     }
     json_object_sset_new(node, "type", type);
 
-    struct timespec ts = { .tv_nsec = UTIME_NOW };
+    struct timespec  ts;
-    json_t *date_added = NULL;
+    json_t          *date_added;
-    if (unlikely(0 != build_tsnode(&ts, &date_added))) {
+    xgetrealtime(&ts);
-        return -EIO;
+    build_tsnode(&ts, &date_added);
-    }
     json_object_sset_new(node, "date_added", date_added);
     json_object_sset_new(node, "date_last_used", json_sstring("0"));
 
@@ -547,23 +543,18 @@ init_iconv (
     return 0;
 }
 
-static int
+static void
 node_mtime_now (
     json_t  *node,
     json_t **tsnode_ptr
 ) {
-    struct timespec ts = { .tv_nsec = UTIME_NOW };
+    json_t *tsnode;
-
+    build_tsnode(NULL, &tsnode);
-    json_t *tsnode = NULL;
-    if (unlikely(0 != build_tsnode(&ts, &tsnode))) {
-        return -1;
-    }
     json_object_sset_new(node, "date_modified", tsnode);
 
     if (tsnode_ptr != NULL) {
         *tsnode_ptr = tsnode;
     }
-    return 0;
 }
 
 static int
@@ -586,13 +577,12 @@ parse_mkfsopts (
 
 static int
 store_new (
-    struct timespec  *btime,
+    struct timespec const  *btime,
-    json_t          **store_ptr
+    json_t                **store_ptr
 ) {
-    json_t *date_added = NULL;
+    json_t *date_added;
-    if (unlikely(0 != build_tsnode(btime, &date_added))) {
+    build_tsnode(btime, &date_added);
-        return -1;
+
-    }
     json_t *children = json_array();
     json_t *type     = json_sstring("folder");
     json_t *zero_str = json_sstring("0");
@@ -676,31 +666,6 @@ update_guid (
     memcpy(entry->guid, guid, UUID_LEN);
 }
 
-static int
-update_node_ts (
-    json_t          *node,
-    struct timespec *times
-) {
-    json_t *ts_node = json_object_sget(node, "date_last_used");
-    if (unlikely(0 != build_tsnode(&times[0], &ts_node))) {
-        return -1;
-    }
-
-    ts_node = json_object_sget(node, "date_modified");
-    if (ts_node == NULL) {
-        ts_node = json_object_sget(node, "date_added");
-        if (unlikely(ts_node == NULL)) {
-            return -1;
-        }
-        json_object_sset_copy(node, "date_modified", ts_node);
-    }
-    if (unlikely(0 != build_tsnode(&times[1], &ts_node))) {
-        return -1;
-    }
-
-    return 0;
-}
-
 #endif  /* defined(BOOKMARKFS_BACKEND_CHROMIUM_WRITE) */
 
 static int
@@ -733,11 +698,9 @@ static int
 build_maps (
     struct backend_ctx *ctx
 ) {
-    json_t *ts_node = NULL;
+    json_t *ts_node;
-    struct timespec now = { .tv_nsec = UTIME_NOW };
+    build_tsnode(NULL, &ts_node);
-    if (unlikely(0 != build_tsnode(&now, &ts_node))) {
+
-        return -1;
-    }
     json_t *root     = json_object();
     json_t *children = json_array();
     json_object_sset_new(root, "children", children);
@@ -798,60 +761,30 @@ build_maps (
     return -1;
 }
 
-static int
+static void
-build_ts (
+build_tsnode (
-    struct timespec *ts,
+    struct timespec const  *ts,
-    char            *buf,
+    json_t                **node_ptr
-    size_t           buf_len
 ) {
-    if (ts->tv_nsec == UTIME_OMIT) {
+    struct timespec now;
-        return 0;
+    if (ts == NULL) {
-    }
+        xgetrealtime(&now);
-    if (ts->tv_nsec == UTIME_NOW) {
+        ts = &now;
-        xgetrealtime(ts);
     }
 
     // XXX: May overflow if system time is badly wrong,
     // but don't bother to check.
-    time_t secs  = ts->tv_sec + EPOCH_DIFF;
+    time_t  secs      = ts->tv_sec + EPOCH_DIFF;
-    long   nsecs = ts->tv_nsec;
+    int64_t microsecs = secs * 1000000 + ts->tv_nsec / 1000;
-
+    if (microsecs < 0) {
-    long microsecs = nsecs / 1000;
+        microsecs = 0;
-    if (unlikely(microsecs >= 1000000)) {
-        secs      += microsecs / 1000000;
-        microsecs %= 1000000;
     }
 
-    int nbytes = snprintf(buf, buf_len, "%" PRIuMAX "%06ld",
-            (uintmax_t)secs, microsecs);
-    if (unlikely(nbytes < 0) || unlikely((size_t)nbytes >= buf_len)) {
-        return -1;
-    }
-    return nbytes;
-}
-
-static int
-build_tsnode (
-    struct timespec  *ts,
-    json_t          **node_ptr
-) {
     char buf[32];
-    int nbytes = build_ts(ts, buf, 32);
+    int nbytes = snprintf(buf, sizeof(buf), "%" PRIi64, microsecs);
-    if (unlikely(nbytes < 0)) {
+    xassert(nbytes > 0 && (size_t)nbytes < sizeof(buf));
-        return -1;
-    }
-    if (nbytes == 0) {
-        // UTIME_OMIT
-        return 0;
-    }
 
-    json_t *node = *node_ptr;
+    *node_ptr = json_stringn_nocheck(buf, nbytes);
-    if (node == NULL) {
-        *node_ptr = json_stringn_nocheck(buf, nbytes);
-    } else {
-        json_string_setn_nocheck(node, buf, nbytes);
-    }
-    return 0;
 }
 
 static void
@@ -2132,9 +2065,8 @@ static int
 backend_mkfs (
     struct bookmarkfs_backend_conf const *conf
 ) {
-    struct parsed_mkfsopts opts = {
+    struct parsed_mkfsopts opts;
-        .btime = { .tv_nsec = UTIME_NOW },
+    xgetrealtime(&opts.btime);
-    };
     if (0 != parse_mkfsopts(conf->opts, &opts)) {
         return -1;
     }
@@ -2253,9 +2185,7 @@ bookmark_create (
     memcpy(entry->guid, guid, UUID_LEN);
     ctx->dirty = DIRTY_LEVEL_DATA;
 
-    if (unlikely(0 != node_mtime_now(parent_entry->node, NULL))) {
+    node_mtime_now(parent_entry->node, NULL);
-        return -EIO;
-    }
     return 0;
 }
 
@@ -2319,9 +2249,7 @@ bookmark_delete (
 
     ctx->dirty = DIRTY_LEVEL_DATA;
 
-    if (unlikely(0 != node_mtime_now(parent_entry->node, NULL))) {
+    node_mtime_now(parent_entry->node, NULL);
-        return -EIO;
-    }
     return 0;
 }
 
@@ -2410,9 +2338,7 @@ bookmark_permute (
     }
     ctx->dirty = DIRTY_LEVEL_DATA;
 
-    if (unlikely(0 != node_mtime_now(parent_entry->node, NULL))) {
+    node_mtime_now(parent_entry->node, NULL);
-        return -EIO;
-    }
     return 0;
 }
 
@@ -2547,9 +2473,7 @@ bookmark_rename (
     ctx->dirty = DIRTY_LEVEL_DATA;
 
     json_t *tsnode_now;
-    if (unlikely(0 != node_mtime_now(old_parent->node, &tsnode_now))) {
+    node_mtime_now(old_parent->node, &tsnode_now);
-        return -EIO;
-    }
     if (old_parent != new_parent) {
         json_object_sset_copy(new_parent->node, "date_modified", tsnode_now);
     }
@@ -2587,12 +2511,18 @@ bookmark_set (
     if (entry->parent_id == BOOKMARKS_ROOT_ID) {
         return -EPERM;
     }
+    json_t *node = entry->node;
 
-    if (flags & BOOKMARK_FLAG(SET_TIME)) {
+    if (flags & BOOKMARK_FLAG(SET_ATIME, SET_MTIME)) {
-        // Without UTIME_NOW, it is safe to cast away the const qualifier.
+        struct timespec const *times = val;
-        struct timespec *times = (struct timespec *)val;
+        json_t *ts_node;
-        if (unlikely(0 != update_node_ts(entry->node, times))) {
+        if (flags & BOOKMARK_FLAG(SET_ATIME)) {
-            return -EIO;
+            build_tsnode(&times[0], &ts_node);
+            json_object_sset_new(node, "date_last_used", ts_node);
+        }
+        if (flags & BOOKMARK_FLAG(SET_MTIME)) {
+            build_tsnode(&times[1], &ts_node);
+            json_object_sset_new(node, "date_modified", ts_node);
         }
         if (ctx->dirty < DIRTY_LEVEL_METADATA) {
             ctx->dirty = DIRTY_LEVEL_METADATA;
@@ -2601,8 +2531,7 @@ bookmark_set (
     }
 
     json_t *val_node;
-    int key_type = get_xattr_val(entry->node, xattr_name, ctx->flags,
+    int key_type = get_xattr_val(node, xattr_name, ctx->flags, &val_node);
-            &val_node);
     if (key_type < 0) {
         return key_type;
     }
@@ -2649,9 +2578,7 @@ bookmark_set (
     ctx->dirty = DIRTY_LEVEL_DATA;
 
     if (key_type != BM_XATTR_NULL && ctx->flags & BOOKMARKFS_BACKEND_CTIME) {
-        if (unlikely(0 != node_mtime_now(entry->node, NULL))) {
+        node_mtime_now(node, NULL);
-            return -EIO;
-        }
     }
     return 0;
 }

src/backend_firefox.c

@@ -35,7 +35,6 @@
 #include <string.h>
 
 #include <fcntl.h>
-#include <sys/stat.h>
 #include <unistd.h>
 
 #ifdef BOOKMARKFS_BACKEND_FIREFOX_WRITE
@@ -2039,9 +2038,6 @@ static int64_t
 timespec_to_usecs (
     struct timespec const *ts
 ) {
-    if (ts->tv_nsec == UTIME_OMIT) {
-        return -1;
-    }
     int64_t microsecs = ts->tv_sec * 1000000 + ts->tv_nsec / 1000;
 
     if (microsecs < 0) {
@@ -3676,13 +3672,15 @@ bookmark_set (
     };
 
     int xattr_id = MOZBM_XATTR_START;
-    if (flags & BOOKMARK_FLAG(SET_TIME)) {
+    if (flags & BOOKMARK_FLAG(SET_ATIME, SET_MTIME)) {
         struct timespec const *times = val;
-        place_cols.last_visit_date = timespec_to_usecs(&times[0]);
+        if (flags & BOOKMARK_FLAG(SET_ATIME)) {
-        bm_cols.last_modified      = timespec_to_usecs(&times[1]);
+            place_cols.last_visit_date = timespec_to_usecs(&times[0]);
-        if (place_cols.last_visit_date >= 0) {
             --xattr_id;
         }
+        if (flags & BOOKMARK_FLAG(SET_MTIME)) {
+            bm_cols.last_modified = timespec_to_usecs(&times[1]);
+        }
     } else {
         xattr_id = get_xattr_id(xattr_name, ctx->flags);
         switch (xattr_id) {

src/fs_ops.c

@@ -440,11 +440,10 @@ bm_do_write (
         return status;
     }
 
-    struct timespec const times[] = {
+    struct timespec const times[2] = {
-        { .tv_nsec = UTIME_OMIT },
+        [1] = fh->mtime,
-        fh->mtime,
     };
-    uint32_t set_flags = BOOKMARK_FLAG(SET_TIME);
+    uint32_t set_flags = BOOKMARK_FLAG(SET_MTIME);
     status = BACKEND_CALL(bookmark_set, id, NULL, set_flags, times, 0);
     if (status < 0) {
         return status;
@@ -1099,23 +1098,28 @@ bm_setattr (
             mask |= TO_SET(MTIME, MTIME_NOW);
         }
 
-        struct timespec ts = { .tv_nsec = UTIME_OMIT };
+        struct timespec now = { 0 };
         if (mask & TO_SET(ATIME_NOW, MTIME_NOW)) {
-            xgetrealtime(&ts);
+            xgetrealtime(&now);
         }
 
-        struct timespec times[] = { ts, ts };
+        uint32_t set_flags = 0;
-        if (TO_SET(ATIME) == (mask & TO_SET(ATIME, ATIME_NOW))) {
-            times[0] = stat_buf->st_atim;
-        }
-        if (TO_SET(MTIME) == (mask & TO_SET(MTIME, MTIME_NOW))) {
-            times[1] = stat_buf->st_mtim;
-        }
-
-        uint32_t set_flags = BOOKMARK_FLAG(SET_TIME);
         if (is_tag) {
             set_flags |= BOOKMARKFS_BOOKMARK_TYPE(TAG);
         }
+        struct timespec times[] = { now, now };
+        if (mask & TO_SET(ATIME)) {
+            if (!(mask & TO_SET(ATIME_NOW))) {
+                times[0] = stat_buf->st_atim;
+            }
+            set_flags |= BOOKMARK_FLAG(SET_ATIME);
+        }
+        if (mask & TO_SET(MTIME)) {
+            if (!(mask & TO_SET(MTIME_NOW))) {
+                times[1] = stat_buf->st_mtim;
+            }
+            set_flags |= BOOKMARK_FLAG(SET_MTIME);
+        }
         int status = BACKEND_CALL(bookmark_set, id, NULL, set_flags, times, 0);
         if (status < 0) {
             return status;