From fd121dc2e212c075bdc47f1c64374315bf4f8e1a Mon Sep 17 00:00:00 2001
From: Christopher Haster <chaster@utexas.edu>
Date: Mon, 9 Jul 2018 14:51:57 -0500
Subject: [PATCH] Dropped "has id" bit encoding in favor of invalid id

I've been trying to keep tag types organized with an encoding that hints
if a tag uses its id field for file ids. However this seem to have been
a mistake. Using a null id of 0x3ff greatly simplified quite a bit of
the logic around managing file related tags.

The downside is one less id we can use, but if we look at the encoding
cost, donating one full bit costs us 2^9 id permutations vs 1 id
permutation. So even if we had a perfect encoding it's in our favor to
use a null id. The cost of null ids is code size, but with the
complexity around figuring out if a type used it's id or not it just
works out better to use a null id.
---
 lfs.c              | 25 +++++++++----------------
 lfs.h              |  2 +-
 tests/test_move.sh | 12 ++++++------
 3 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/lfs.c b/lfs.c
index 708ae36..8912407 100644
--- a/lfs.c
+++ b/lfs.c
@@ -439,10 +439,6 @@ static inline bool lfs_tag_isuser(lfs_tag_t tag) {
     return (tag & 0x40000000);
 }
 
-static inline bool lfs_tag_hasid(lfs_tag_t tag) {
-    return (tag & 0x60000000) != 0x20000000;
-}
-
 static inline uint16_t lfs_tag_type(lfs_tag_t tag) {
     return (tag & 0x7fc00000) >> 22;
 }
@@ -498,14 +494,14 @@ static int lfs_commit_move(lfs_t *lfs, struct lfs_commit *commit,
 static int lfs_commit_commit(lfs_t *lfs,
         struct lfs_commit *commit, lfs_mattr_t attr) {
     // filter out ids
-    if (lfs_tag_hasid(attr.tag) && (
+    if (lfs_tag_id(attr.tag) < 0x3ff && (
             lfs_tag_id(attr.tag) < commit->filter.begin ||
             lfs_tag_id(attr.tag) >= commit->filter.end)) {
         return 0;
     }
 
     // special cases
-    if (lfs_tag_type(attr.tag) == LFS_FROM_MOVE) {
+    if (lfs_tag_type(attr.tag) == LFS_FROM_DIR) {
         return lfs_commit_move(lfs, commit,
                 lfs_tag_size(attr.tag), lfs_tag_id(attr.tag),
                 attr.u.dir, NULL); 
@@ -657,8 +653,7 @@ static int lfs_commit_movescan(lfs_t *lfs, void *p, lfs_mattr_t attr) {
         return 0;
     }
 
-    // TODO need this if 0x3ff is required?
-    if (!lfs_tag_hasid(attr.tag) || lfs_tag_id(attr.tag) != move->id.from) {
+    if (lfs_tag_id(attr.tag) != move->id.from) {
         // ignore non-matching ids
         return 0;
     }
@@ -721,9 +716,8 @@ static int lfs_commit_globals(lfs_t *lfs, struct lfs_commit *commit,
     // TODO check performance/complexity of different strategies here
     lfs_globals_t res = lfs_globals_xor(source, diff);
     int err = lfs_commit_commit(lfs, commit, (lfs_mattr_t){
-            lfs_mktag(LFS_TYPE_GLOBALS,
-                res.move.id, sizeof(res.move.pair)),
-                .u.buffer=res.move.pair});
+            lfs_mktag(LFS_TYPE_GLOBALS, 0x3ff, sizeof(res)),
+            .u.buffer=&res});
     if (err) {
         return err;
     }
@@ -899,15 +893,14 @@ static int lfs_dir_fetchwith(lfs_t *lfs,
                         return err;
                     }
                 } else if (lfs_tag_type(tag) == LFS_TYPE_GLOBALS) {
-                    temp.globals.move.id = lfs_tag_id(tag);
                     err = lfs_bd_read(lfs, temp.pair[0], off+sizeof(tag),
-                            &temp.globals.move.pair,
-                            sizeof(temp.globals.move.pair));
+                            &temp.globals, sizeof(temp.globals));
                     if (err) {
                         return err;
                     }
                 } else {
-                    if (lfs_tag_hasid(tag) && lfs_tag_id(tag) >= temp.count) {
+                    if (lfs_tag_id(tag) < 0x3ff &&
+                            lfs_tag_id(tag) >= temp.count) {
                         temp.count = lfs_tag_id(tag)+1;
                     }
 
@@ -2822,7 +2815,7 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
     err = lfs_dir_commit(lfs, &newcwd, &(lfs_mattrlist_t){
             {lfs_mktag(oldtype, newid, strlen(newpath)),
                 .u.buffer=(void*)newpath}, &(lfs_mattrlist_t){
-            {lfs_mktag(LFS_FROM_MOVE, newid, oldid),
+            {lfs_mktag(LFS_FROM_DIR, newid, oldid),
                 .u.dir=&oldcwd}}});
     if (err) {
         return err;
diff --git a/lfs.h b/lfs.h
index cef224b..5c00a12 100644
--- a/lfs.h
+++ b/lfs.h
@@ -118,7 +118,7 @@ enum lfs_type {
     // internal chip sources
     LFS_FROM_REGION     = 0x000,
     LFS_FROM_DISK       = 0x200,
-    LFS_FROM_MOVE       = 0x030,
+    LFS_FROM_DIR        = 0x030,
 };
 
 // File open flags
diff --git a/tests/test_move.sh b/tests/test_move.sh
index 5b851c5..790706f 100755
--- a/tests/test_move.sh
+++ b/tests/test_move.sh
@@ -59,7 +59,7 @@ tests/test.py << TEST
     lfs_rename(&lfs, "b/hello", "c/hello") => 0;
     lfs_unmount(&lfs) => 0;
 TEST
-truncate -s-11 blocks/6
+truncate -s-7 blocks/6
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_dir_open(&lfs, &dir[0], "b") => 0;
@@ -86,8 +86,8 @@ tests/test.py << TEST
     lfs_rename(&lfs, "c/hello", "d/hello") => 0;
     lfs_unmount(&lfs) => 0;
 TEST
-truncate -s-11 blocks/8
-truncate -s-11 blocks/a
+truncate -s-7 blocks/8
+truncate -s-7 blocks/a
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_dir_open(&lfs, &dir[0], "c") => 0;
@@ -166,7 +166,7 @@ tests/test.py << TEST
     lfs_rename(&lfs, "b/hi", "c/hi") => 0;
     lfs_unmount(&lfs) => 0;
 TEST
-truncate -s-11 blocks/7
+truncate -s-7 blocks/7
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_dir_open(&lfs, &dir[0], "b") => 0;
@@ -193,8 +193,8 @@ tests/test.py << TEST
     lfs_rename(&lfs, "c/hi", "d/hi") => 0;
     lfs_unmount(&lfs) => 0;
 TEST
-truncate -s-11 blocks/9
-truncate -s-11 blocks/b
+truncate -s-7 blocks/9
+truncate -s-7 blocks/b
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_dir_open(&lfs, &dir[0], "c") => 0;