Don't rely on implicit bool-cast in lfs_gstate_hasmove()

[johnernberg]

bool could be defined to be an (unsigned) char, which causes the expression in lfs_gstate_hasmove() to always evaluate to false since the truth:y value is kept in the upper 8 bits of the uint16_t which will get truncated in the implicit cast.

This will further bring with it that the compiler may then determine that lfs_gstate_hasmove() can never return true, and while optimizing removing the lfs_dir_commit() of oldcwd in lfs_rawrename(). Leading to a corrupt file system where files and directories are duplicated.

Signed-off-by: John Ernberg john.ernberg@actia.se

[johnernberg]

I am unsure if the lfs_gstate_hasorphans() should get the same treatment.

[geky]

Hi @johnernberg, thanks for creating a PR.

I could merge this, but it's a fragile solution. littlefs is a c99 project and is tested with c99 compilers, where bool as a type is strictly 0 or 1 and the type cast does the truthy conversion correctly.

Even if we fix this here, it's easy for this behavior to be reintroduced in a later commit because our tests will never catch it.

An alternative option is to add an assert against uint8_t bools in lfs_mount, thoughts?

// bool must be a truthy-preserving type
LFS_ASSERT((bool)0x80000000);

If your compiler is limited to c89, that's a different issue. I think c89 support would be valuable eventually, but it's a bit out of scope for now.

[johnernberg]

Hi @geky

Thank you for the response and feedback, I agree with your reluctance to merge this as you explained it will definitely be fragile.

The issue was found in the combination of NuttX and LittleFS as NuttX by default compiles with C89. After discussion with my colleague we decided to open an issue with NuttX as this now sounds like an integration problem. It may be easier to spot such integration issues with the proposed assert.

Would you like us to attempt such a patch?

[geky]

That's unfortunate, NuttX compatibility would be additional motivation for a c89 port.

I'll be following the NuttX issue to see what the conclusion is.

Would you like us to attempt such a patch?

Ah sorry I went ahead and created #801 before reading this comment closely. Let me know if it looks good to you.

[johnernberg]

Closing in favor of #801