Reduced when custom attributse are written to strictly when files are dirty

This is a bit of a complicated area for the custom-attribute API without
much precedent. littlefs allows users to provide custom attributes in
the lfs_file_config struct, which get written along with other file
metadata.

Sounds great on paper, but the devil is in the details. When does the
metadata actually get written?

What about this case?

    lfs_file_opencfg(lfs, file, "path", LFS_O_WRONLY, cfg_with_attrs);
    lfs_file_close(lfs, file); // does not write metadata

This normally doesn't write out metadata! We've opened the file for
writing, but made no changes, so normally littlefs doesn't bother to
commit anything to disk.

Before, as a courtesy, littlefs marked the file as dirty if it noticed
the file was opened for writing with custom attributes, but this is
inaccurate could to leave to problems after a file is synced:

    lfs_file_opencfg(lfs, file, "path", LFS_O_WRONLY, cfg_with_attrs);
    lfs_file_sync(lfs, file);
    change_attrs();
    lfs_file_close(lfs, file); // does not write metadata

Unfortunately, it isn't easy to know when metadata needs to be written.
Custom attributes are provided as read-only pointers to buffers which
may be updated without additional filesystem calls, this means we don't
know if custom attributes have actually changed on the device side. If
they haven't changed, writing out metadata on every sync would be
wasteful.

Another solution would be to compare our device-side attributes with
the disk-side attributes every sync, but that would be even more
expensive.

---

So for now, the simpliest and most efficient solution wins. Custom
attributes attached to open files, are not written unless the file data
itself changes.

Note that explicit calls to lfs_setattr always update on-disk
attributes, and opening a file with LFS_O_CREATE | LFS_O_TRUNC will also
always update the on-disk attributes (though not with just LFS_O_CREAT!).

There are a few ways we could provide an API that manually forces a write
of custom attributes, such as lfs_file_setattr, though without dynamic
memory, providing these APIs gets a bit complicated. So for now we will
see if users run into issues with the current scheme.

lfs.c

@@ -2506,11 +2506,12 @@ static int lfs_file_rawopencfg(lfs_t *lfs, lfs_file_t *file,
         }
 
         // get next slot and create entry to remember name
-        // TODO commit with attrs?
         err = lfs_dir_commit(lfs, &file->m, LFS_MKATTRS(
                 {LFS_MKTAG(LFS_TYPE_CREATE, file->id, 0), NULL},
                 {LFS_MKTAG(LFS_TYPE_REG, file->id, nlen), path},
-                {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, file->id, 0), NULL}));
+                {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, file->id, 0), NULL},
+                {LFS_MKTAG(LFS_FROM_USERATTRS, file->id,
+                    file->cfg->attr_count), file->cfg->attrs}));
         if (err) {
             err = LFS_ERR_NAMETOOLONG;
             goto cleanup;
@@ -2563,9 +2564,6 @@ static int lfs_file_rawopencfg(lfs_t *lfs, lfs_file_t *file,
                 err = LFS_ERR_NOSPC;
                 goto cleanup;
             }
-
-            // TODO remove this?
-            file->flags |= LFS_F_DIRTY;
         }
 #endif
     }

tests/test_attrs.toml

@@ -16,41 +16,41 @@ code = '''
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  6) => 6;
     lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "bbbbbb", 6) => 0;
+    assert(memcmp(buffer+4,  "bbbbbb", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     lfs_setattr(&lfs, "hello", 'B', "", 0) => 0;
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  6) => 0;
     lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",         4) => 0;
+    assert(memcmp(buffer,    "aaaa",         4) == 0);
-    memcmp(buffer+4,  "\0\0\0\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "\0\0\0\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",        5) => 0;
+    assert(memcmp(buffer+10, "ccccc",        5) == 0);
 
     lfs_removeattr(&lfs, "hello", 'B') => 0;
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  6) => LFS_ERR_NOATTR;
     lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",         4) => 0;
+    assert(memcmp(buffer,    "aaaa",         4) == 0);
-    memcmp(buffer+4,  "\0\0\0\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "\0\0\0\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",        5) => 0;
+    assert(memcmp(buffer+10, "ccccc",        5) == 0);
 
     lfs_setattr(&lfs, "hello", 'B', "dddddd", 6) => 0;
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  6) => 6;
     lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "dddddd", 6) => 0;
+    assert(memcmp(buffer+4,  "dddddd", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     lfs_setattr(&lfs, "hello", 'B', "eee", 3) => 0;
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  6) => 3;
     lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "eee\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "eee\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",     5) => 0;
+    assert(memcmp(buffer+10, "ccccc",     5) == 0);
 
     lfs_setattr(&lfs, "hello", 'A', buffer, LFS_ATTR_MAX+1) => LFS_ERR_NOSPC;
     lfs_setattr(&lfs, "hello", 'B', "fffffffff", 9) => 0;
@@ -65,13 +65,13 @@ code = '''
     lfs_getattr(&lfs, "hello", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "hello", 'B', buffer+4,  9) => 9;
     lfs_getattr(&lfs, "hello", 'C', buffer+13, 5) => 5;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "fffffffff", 9) => 0;
+    assert(memcmp(buffer+4,  "fffffffff", 9) == 0);
-    memcmp(buffer+13, "ccccc",     5) => 0;
+    assert(memcmp(buffer+13, "ccccc",     5) == 0);
 
     lfs_file_open(&lfs, &file, "hello/hello", LFS_O_RDONLY) => 0;
     lfs_file_read(&lfs, &file, buffer, sizeof(buffer)) => strlen("hello");
-    memcmp(buffer, "hello", strlen("hello")) => 0;
+    assert(memcmp(buffer, "hello", strlen("hello")) == 0);
     lfs_file_close(&lfs, &file);
     lfs_unmount(&lfs) => 0;
 '''
@@ -94,41 +94,41 @@ code = '''
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  6) => 6;
     lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "bbbbbb", 6) => 0;
+    assert(memcmp(buffer+4,  "bbbbbb", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     lfs_setattr(&lfs, "/", 'B', "", 0) => 0;
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  6) => 0;
     lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",         4) => 0;
+    assert(memcmp(buffer,    "aaaa",         4) == 0);
-    memcmp(buffer+4,  "\0\0\0\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "\0\0\0\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",        5) => 0;
+    assert(memcmp(buffer+10, "ccccc",        5) == 0);
 
     lfs_removeattr(&lfs, "/", 'B') => 0;
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  6) => LFS_ERR_NOATTR;
     lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",         4) => 0;
+    assert(memcmp(buffer,    "aaaa",         4) == 0);
-    memcmp(buffer+4,  "\0\0\0\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "\0\0\0\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",        5) => 0;
+    assert(memcmp(buffer+10, "ccccc",        5) == 0);
 
     lfs_setattr(&lfs, "/", 'B', "dddddd", 6) => 0;
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  6) => 6;
     lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "dddddd", 6) => 0;
+    assert(memcmp(buffer+4,  "dddddd", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     lfs_setattr(&lfs, "/", 'B', "eee", 3) => 0;
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  6) => 3;
     lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "eee\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "eee\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",     5) => 0;
+    assert(memcmp(buffer+10, "ccccc",     5) == 0);
 
     lfs_setattr(&lfs, "/", 'A', buffer, LFS_ATTR_MAX+1) => LFS_ERR_NOSPC;
     lfs_setattr(&lfs, "/", 'B', "fffffffff", 9) => 0;
@@ -142,13 +142,13 @@ code = '''
     lfs_getattr(&lfs, "/", 'A', buffer,    4) => 4;
     lfs_getattr(&lfs, "/", 'B', buffer+4,  9) => 9;
     lfs_getattr(&lfs, "/", 'C', buffer+13, 5) => 5;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "fffffffff", 9) => 0;
+    assert(memcmp(buffer+4,  "fffffffff", 9) == 0);
-    memcmp(buffer+13, "ccccc",     5) => 0;
+    assert(memcmp(buffer+13, "ccccc",     5) == 0);
 
     lfs_file_open(&lfs, &file, "hello/hello", LFS_O_RDONLY) => 0;
     lfs_file_read(&lfs, &file, buffer, sizeof(buffer)) => strlen("hello");
-    memcmp(buffer, "hello", strlen("hello")) => 0;
+    assert(memcmp(buffer, "hello", strlen("hello")) == 0);
     lfs_file_close(&lfs, &file);
     lfs_unmount(&lfs) => 0;
 '''
@@ -176,48 +176,52 @@ code = '''
     memcpy(buffer,    "aaaa",   4);
     memcpy(buffer+4,  "bbbbbb", 6);
     memcpy(buffer+10, "ccccc",  5);
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_close(&lfs, &file) => 0;
     memset(buffer, 0, 15);
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0;
     lfs_file_close(&lfs, &file) => 0;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "bbbbbb", 6) => 0;
+    assert(memcmp(buffer+4,  "bbbbbb", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     attrs1[1].size = 0;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1) => 0;
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_close(&lfs, &file) => 0;
     memset(buffer, 0, 15);
     attrs1[1].size = 6;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0;
     lfs_file_close(&lfs, &file) => 0;
-    memcmp(buffer,    "aaaa",         4) => 0;
+    assert(memcmp(buffer,    "aaaa",         4) == 0);
-    memcmp(buffer+4,  "\0\0\0\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "\0\0\0\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",        5) => 0;
+    assert(memcmp(buffer+10, "ccccc",        5) == 0);
 
     attrs1[1].size = 6;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1) => 0;
     memcpy(buffer+4,  "dddddd", 6);
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_close(&lfs, &file) => 0;
     memset(buffer, 0, 15);
     attrs1[1].size = 6;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0;
     lfs_file_close(&lfs, &file) => 0;
-    memcmp(buffer,    "aaaa",   4) => 0;
+    assert(memcmp(buffer,    "aaaa",   4) == 0);
-    memcmp(buffer+4,  "dddddd", 6) => 0;
+    assert(memcmp(buffer+4,  "dddddd", 6) == 0);
-    memcmp(buffer+10, "ccccc",  5) => 0;
+    assert(memcmp(buffer+10, "ccccc",  5) == 0);
 
     attrs1[1].size = 3;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1) => 0;
     memcpy(buffer+4,  "eee", 3);
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_close(&lfs, &file) => 0;
     memset(buffer, 0, 15);
     attrs1[1].size = 6;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0;
     lfs_file_close(&lfs, &file) => 0;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "eee\0\0\0", 6) => 0;
+    assert(memcmp(buffer+4,  "eee\0\0\0", 6) == 0);
-    memcmp(buffer+10, "ccccc",     5) => 0;
+    assert(memcmp(buffer+10, "ccccc",     5) == 0);
 
     attrs1[0].size = LFS_ATTR_MAX+1;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1)
@@ -231,6 +235,7 @@ code = '''
     struct lfs_file_config cfg2 = {.attrs=attrs2, .attr_count=3};
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDWR, &cfg2) => 0;
     memcpy(buffer+4,  "fffffffff", 9);
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_close(&lfs, &file) => 0;
     attrs1[0].size = 4;
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0;
@@ -249,13 +254,13 @@ code = '''
 
     lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg3) => 0;
     lfs_file_close(&lfs, &file) => 0;
-    memcmp(buffer,    "aaaa",      4) => 0;
+    assert(memcmp(buffer,    "aaaa",      4) == 0);
-    memcmp(buffer+4,  "fffffffff", 9) => 0;
+    assert(memcmp(buffer+4,  "fffffffff", 9) == 0);
-    memcmp(buffer+13, "ccccc",     5) => 0;
+    assert(memcmp(buffer+13, "ccccc",     5) == 0);
 
     lfs_file_open(&lfs, &file, "hello/hello", LFS_O_RDONLY) => 0;
     lfs_file_read(&lfs, &file, buffer, sizeof(buffer)) => strlen("hello");
-    memcmp(buffer, "hello", strlen("hello")) => 0;
+    assert(memcmp(buffer, "hillo", strlen("hello")) == 0);
     lfs_file_close(&lfs, &file);
     lfs_unmount(&lfs) => 0;
 '''
@@ -287,17 +292,18 @@ code = '''
     lfs_getattr(&lfs, "hello/hello", 'B', buffer,    9) => 9;
     lfs_getattr(&lfs, "hello/hello", 'C', buffer+9,  9) => 5;
     lfs_getattr(&lfs, "hello/hello", 'D', buffer+18, 9) => LFS_ERR_NOATTR;
-    memcmp(buffer,    "fffffffff",          9) => 0;
+    assert(memcmp(buffer,    "fffffffff",          9) == 0);
-    memcmp(buffer+9,  "ccccc\0\0\0\0",      9) => 0;
+    assert(memcmp(buffer+9,  "ccccc\0\0\0\0",      9) == 0);
-    memcmp(buffer+18, "\0\0\0\0\0\0\0\0\0", 9) => 0;
+    assert(memcmp(buffer+18, "\0\0\0\0\0\0\0\0\0", 9) == 0);
 
+    lfs_file_write(&lfs, &file, "hi", 2) => 2;
     lfs_file_sync(&lfs, &file) => 0;
     lfs_getattr(&lfs, "hello/hello", 'B', buffer,    9) => 4;
     lfs_getattr(&lfs, "hello/hello", 'C', buffer+9,  9) => 0;
     lfs_getattr(&lfs, "hello/hello", 'D', buffer+18, 9) => 4;
-    memcmp(buffer,    "gggg\0\0\0\0\0",     9) => 0;
+    assert(memcmp(buffer,    "gggg\0\0\0\0\0",     9) == 0);
-    memcmp(buffer+9,  "\0\0\0\0\0\0\0\0\0", 9) => 0;
+    assert(memcmp(buffer+9,  "\0\0\0\0\0\0\0\0\0", 9) == 0);
-    memcmp(buffer+18, "hhhh\0\0\0\0\0",     9) => 0;
+    assert(memcmp(buffer+18, "hhhh\0\0\0\0\0",     9) == 0);
 
     lfs_file_close(&lfs, &file) => 0;
     lfs_unmount(&lfs) => 0;