fix: set storfs size_t type alias to just standard size_t#27
fix: set storfs size_t type alias to just standard size_t#27prashantrahul141 wants to merge 1 commit into
Conversation
|
This fixes the seg fault in #26. I will write tests which will make sure to catch all these locally or in the ci. |
|
|
||
| /** @brief Describes the page location */ | ||
| typedef uint32_t storfs_page_t; | ||
| typedef size_t storfs_page_t; |
There was a problem hiding this comment.
This should stay at uint32_t. In the snode work this is expected to be 4 bytes to fit properly in the calculations of the SNode struct size (see: https://github.com/matt001k/STORfs/blob/e6d535fd008dd13e6c706cb7577d8dbaf23c5342/src/snode.h#L31C1-L60C70). This value should be locked in at either uint32_t or uint64_t, doing so will ensure that there is consistency across platforms and the FS stays compatible across platforms agnostic of the storage device.
There was a problem hiding this comment.
This seems to be the cause of seg fault in #26. Using uint32_t triggers the seg fault while keeping it to uint64_t doesn't? Think we should pin this to uint64_t?
The performance overhead of keeping it to uint64_t on 32bit systems wouldn't be much I think.
There was a problem hiding this comment.
All of the file access functions (fputs, fgets, open, etc.) will need to be updated once the SNode Directory Hash Table is implemented, rendering the current implementations stale.
There was a problem hiding this comment.
You can think of bitmap.c, snode.c and soon hash.c as the new engine to this car we are building 😄
I am extremely close to finishing snode.c then I will have time to write a design document for hash.c.
Following that, all of the file access functions can easily be re-written.
There was a problem hiding this comment.
I see, is it possible to keep snode's size calculation dynamic? maybe via sizeof? This will allow us to keep storfs_page_t as size_t and the size of snode will also be consistent.
|
|
||
| /** @brief Filesize alias for filesize register */ | ||
| typedef uint32_t storfs_file_size_t; | ||
| typedef size_t storfs_file_size_t; |
There was a problem hiding this comment.
This may be able to be deleted in the future.
prashantrahul141
force-pushed
the
fix/different-size_t
branch
from
d124189 to
dda1cb6
Compare
This patch sets storfs's different size_t type alises to be just size_t for better portability across different word size machines Signed-off-by: Prashant Rahul <prashantrahul141@protonmail.com>
prashantrahul141
force-pushed
the
fix/different-size_t
branch
from
dda1cb6 to
96822a5
Compare
|
Sorry I will be chewing on this one for a bit more as the snode work wraps up. |
|
No problem! please take your time. I am currently writing some unit tests as I mentioned earlier |
2 participants
This patch sets storfs's different size_t type alises to be just size_t for better portability across different word size machines
closes #26
closes #23