Skip to content

Add LFS_READONLY define, to allow smaller builds providing read-only mode #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
geky merged 10 commits into from
Dec 4, 2020
+177 −6
Merged

Add LFS_READONLY define, to allow smaller builds providing read-only mode #480

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8e6826c
Add LFS_READYONLY define, to allow smaller builds providing read-only…
maximevince Oct 28, 2020
46b5033
LFS_READONLY: undef unavailable function declarations altogether
maximevince Nov 17, 2020
882aae5
LFS_READONLY: even less code, assert on write attempts
maximevince Nov 19, 2020
69da107
LFS_READONLY: remove LFS_O_WRONLY and other flags when compiling with…
maximevince Nov 20, 2020
0360629
LFS_READONLY: do not annotate #endif, as requested
maximevince Nov 21, 2020
83fd2e8
LFS_READONLY: move ifdef before comments blocks, rework dangling open…
maximevince Nov 21, 2020
35cc31e
LFS_READONLY: ifdef file flags that are not needed in read-only mode
maximevince Nov 21, 2020
94c9c4c
LFS_READONLY: slight refactor
maximevince Nov 21, 2020
5c4e375
Added read-only build+size reporting to CI
geky Nov 22, 2020
99c960b
LFS_READONLY: ifdef LFS_F_ERRED out as well
maximevince Nov 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
LFS_READONLY: ifdef file flags that are not needed in read-only mode
  • Loading branch information
@maximevince
maximevince committed Nov 21, 2020
commit 35cc31e408d6ff4169ec7e5fe285f35c93cfea0f
4 changes: 2 additions & 2 deletions lfs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3996,6 +3996,7 @@ int lfs_fs_traverseraw(lfs_t *lfs,
}
}

#ifndef LFS_READONLY
// iterate over any open files
for (lfs_file_t *f = (lfs_file_t*)lfs->mlist; f; f = f->next) {
if (f->type != LFS_TYPE_REG) {
Expand All @@ -4010,16 +4011,15 @@ int lfs_fs_traverseraw(lfs_t *lfs,
}
}

#ifndef LFS_READONLY
if ((f->flags & LFS_F_WRITING) && !(f->flags & LFS_F_INLINE)) {
int err = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache,
f->block, f->pos, cb, data);
if (err) {
return err;
}
}
#endif
}
#endif

return 0;
}
Expand Down
4 changes: 3 additions & 1 deletion lfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ enum lfs_open_flags {
#endif

// internally used flags
#ifndef LFS_READONLY
LFS_F_DIRTY = 0x010000, // File does not match storage
Copy link
Contributor

@dpgeorge dpgeorge Nov 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible (and worth it) also #if'ing these constants like LFS_F_DIRTY which are not supposed to be used in read-only mode?

Copy link
Contributor Author

@maximevince maximevince Nov 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was easy for LFS_F_DIRTY and LFS_F_WRITING.

However, I am not sure about LFS_F_ERRED.
It's documented as "An error occurred during write", but it has one non-obvious use at the end of int lfs_file_opencfg().
It seems you could end up there when e.g. lfs_malloc() fails, or when opening a file for read hits an LFS_ERR_NOENT (here) (among other possibilities).
Both of which cases do not seem to be strictly write-related.
So I conclude that this constant is needed in LFS_READONLY mode as well.

This is implemented in 35cc31e

Copy link
Member

@geky geky Nov 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's not actually used in lfs_file_close but in the implicit lfs_file_sync. The purpose of the LFS_F_ERRED flag is to tell lfs_file_close to release resources without writing out to disk. lfs_file_opencfg immediately calls lfs_file_close if an error occurs.

You have the implicit lfs_file_sync removed in LFS_READONLY mode, so it should be safe to remove this line as well.

Copy link
Contributor Author

@maximevince maximevince Nov 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only noticed this comment now. I have ifdefed LFS_F_ERRED out completely. (99c960b)

LFS_F_WRITING = 0x020000, // File has been written since last flush
#endif
LFS_F_READING = 0x040000, // File has been read since last flush
LFS_F_ERRED = 0x080000, // An error occured during write
LFS_F_ERRED = 0x080000, // An error occurred during write
LFS_F_INLINE = 0x100000, // Currently inlined in directory entry
LFS_F_OPENED = 0x200000, // File has been opened
};
Expand Down