API to check if file is currently open

[keck-in-space]

I have a need to check if a file is currently open. Is it OK to do so as indicated below?

lfs_file_t* file;
bool isOpen = ((file->flags & LFS_F_OPENED) == LFS_F_OPENED);

This is a useful check to be made in cases where files are written autonomously and could fail to close before the next attempt to open is made.

LFS defines the LFS_F_OPENED flag as internally used, so I wanted to check if this access was safe, or if there are some conditions when the flag isn't immediately updated.

[geky]

Oh interesting. Could you not use the return value from lfs_file_close to set a flag in your application logic?

The reason LFS_F_OPENED is noted as a internal only is to keep us free to change its behavior in any release. It's often hard enough to fix things while maintaining backwards compatibility with the public API.

[keck-in-space]

Yes, that is one way to handle this. I wanted to avoid tracking it externally if possible, since it would be another variable to keep track of, and we'd need to write to it every time an opencfg or close executed (successfully or otherwise). We're locked in on LFS 2.1.4, if that matters.

[geky]

This is a useful check to be made in cases where files are written autonomously and could fail to close before the next attempt to open is made.

Actually, thinking about it, are you sure this flag gives you this information?

lfs_file_close is a bit strange in that it does two things:

• Syncs data to disk.
• Cleans up resources.

The resource clean up can not fail. If an error occurs on write, the pending data is discarded, but the local resources (file cache, anything allocated) are deallocated and lost. This way common code that does open->write->close does not memory leak if a filesystem error occurs (such as a full disk).

If you need to take recovery action and not lose the data, lfs_file_sync syncs data to disk without cleaning up resources. An lfs_file_sync followed by lfs_file_close is guaranteed to do no extra disk operations.

---

So if an error happens, this should change the state of LFS_F_OPENED. Instead LFS_F_ERRED gets set, which indicates if a write error occured. If it's set, all future writes on the file are discarded.

LFS_F_OPENED is really only used as a debug assertion to help prevent some programming mistakes such as opening a file twice or writing after close. It also may be replaced by #443 which looks like it has the same affect without needing an extra bit.

[keck-in-space]

So if an error happens, this should change the state of LFS_F_OPENED. Instead LFS_F_ERRED gets set, which indicates if a write error occured. If it's set, all future writes on the file are discarded.

Are you saying that if an error happens "this shouldn't change the state of LFS_F_OPENED?" or is it correct as written? If so, I'm not sure I follow.

I think it might be helpful to detail the use case here, since I don't always close the file.

The way we've set up our logging infrastructure we can support two types of logs.

1. Log files - Open-ended logs up to a size cap (lets say 4 MB)
2. Sequenced files - Files with writes governed by flags sent by another subsystem (BEGIN-MIDDLE-END flags, also has a file size cap...)

To improve speed while retaining some file integrity, each subsystem is assigned a sync threshold (every 16KB) and kept open until they reach the file size cap or for sequenced files, see an END flag.

The flow goes like this:

• If the file isn't open, open it.
• Check if the data to write will exceed the size cap. If so, close the file and create a new one and open it.
• Write the data to the file.
• Check if the newly written data means we're at a syncing threshold. If so, execute a sync.
• Check if the sequence flag indicates this is the end of this log file. If so, close the file.

Since the file can either be open or closed at the end of this sequence, I need a way to check if it's open at the start of the sequence so I don't hit an assert by doubly opening the file or attempting to write while the file isn't open. I hope this makes some sense. Thanks!