Skip to content
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

Microfsexp #18

Merged
merged 12 commits into from
May 30, 2023
Merged

Microfsexp #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
840dd87
Merge pull request #1 from timcanham/microfsexp
gjwatney Apr 6, 2023
dc648ce
Update GenericHub documentation with a couple diagrams
gjwatney Apr 10, 2023
f530262
Add MicroFs CopyTest
Apr 13, 2023
cd06591
Merge branch 'microfsexp' of https://github.com/gjwatney/fprime-timca…
Apr 13, 2023
c6c48a6
Add MicroFs MergeTest
Apr 17, 2023
0c05245
Fix MicroFs.cpp build issue with unsigned int
May 2, 2023
58480b1
Fix Warnings in MicroFs unit test
May 2, 2023
e7205c1
Fix all MicroFs unit tests
May 3, 2023
3e2975a
Fix runtime errors with alignment, stack overflow and memory leaks
May 23, 2023
3e2cdb5
Removed unused functions from MicroFs
May 23, 2023
b4ddd60
Add new test
May 24, 2023
454ef9c
Add another random unit test
May 24, 2023
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
1 change: 0 additions & 1 deletion Os/.gitignore
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions Os/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ set(UT_SOURCE_FILES
"${CMAKE_CURRENT_LIST_DIR}/MicroFs/test/ut/MicroFsTest.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MicroFs/test/ut/Tester.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MicroFs/test/ut/MyRules.cpp"
"${CMAKE_CURRENT_LIST_DIR}/MicroFs/test/ut/SimFileSystem.cpp"
)
set (UT_MOD_DEPS STest)
register_fprime_ut("Os_MicroFs")
Expand Down
19 changes: 4 additions & 15 deletions Os/MicroFs/MicroFs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ getFileStateIndex(const char* fileName) {

FwNativeUIntType binIndex;
FwNativeUIntType fileIndex;
char crcExtension;
FwNativeIntType stat = sscanf(fileName,filePathSpec,&binIndex,&fileIndex,&crcExtension);
// crcExtension should be 2 bytes because scanf appends a null character at the end.
char crcExtension[2];
FwNativeIntType stat = sscanf(fileName,filePathSpec,&binIndex,&fileIndex,&crcExtension[0]);
if (stat != 2) {
return -1;
}
Expand Down Expand Up @@ -528,7 +529,7 @@ Status createDirectory(const char* path) {
FW_ASSERT(MicroFsMem);
MicroFsConfig *cfg = static_cast<MicroFsConfig*>(MicroFsMem);

if ((binIndex >= 0) and (binIndex < cfg->numBins)) {
if (binIndex < cfg->numBins) {
return OP_OK;
} else {
return NO_PERMISSION;
Expand Down Expand Up @@ -663,18 +664,6 @@ Status moveFile(const char* originPath, const char* destPath) {

}

Status handleFileError(File::Status fileStatus) {
Status fileSystemStatus = OTHER_ERROR;

return fileSystemStatus;
}

Status initAndCheckFileStats(const char* filePath, struct stat* fileInfo = nullptr) {
FileSystem::Status fs_status = OP_OK;

return fs_status;
}

Status copyFile(const char* originPath, const char* destPath) {

if ((not originPath) or (not destPath)) {
Expand Down
2 changes: 2 additions & 0 deletions Os/MicroFs/MicroFs.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ struct MicroFsBin {

struct MicroFsConfig {
FwNativeUIntType numBins; //!< The number of bins configured. Must be <= than MAX_MICROFS_BINS
FwNativeUIntType filler; //!< filler bytes to ensure 8 byte alignment of this data structure
Copy link

@bocchino bocchino May 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There are several issues here:

  1. This code seems to assume that sizeof(FwNativeUnitType) == 4. We can't assume that. FwNativeUintType does not have a defined architecture-independent size.
  2. We should not be using FwNativeUintType anyway. It doesn't have a defined size.
  3. A better way to ensure alignment in C++11 is to use alignas. If you can't use alignas for some reason, then you can write U8 pad[4] for example to add padding of 4 bytes.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm going to merge it to my fork to get the other changes, then we'll deal with the alignment and native integer issues.

MicroFsBin bins[MAX_MICROFS_BINS]; //!< The bins containing file sizes and numbers of files
};
static_assert(sizeof(MicroFsConfig) % 8 == 0, "Size of MicroFsConfig is not divisible by 8");

//!< set the number of bins in config
void MicroFsSetCfgBins(MicroFsConfig& cfg, const FwNativeUIntType numBins);
Expand Down
27 changes: 26 additions & 1 deletion Os/MicroFs/test/ut/MicroFsTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#define FULL_TEST
#define NUKE_TEST
#define OFF_NOMINAL
#define NEW_TEST
//#define SIM_FILE_TEST

#ifdef FULL_TEST

Expand Down Expand Up @@ -88,8 +90,17 @@ TEST(FileOps, OddTests) {
tester.OddTests();
}

#endif
TEST(FileOps, CopyTest) {
Os::Tester tester;
tester.CopyTest();
}

TEST(FileOps, AppendTest) {
Os::Tester tester;
tester.AppendTest();
}

#endif

#ifdef NUKE_TEST
TEST(FileOps, NukeTest) {
Expand All @@ -105,6 +116,20 @@ TEST(FileOps, OffNominalTests) {
}
#endif

#ifdef SIM_FILE_TEST
TEST(FileOps, SimFileTest) {
Os::Tester tester;
tester.SimFileTest();
}
#endif

#ifdef NEW_TEST
TEST(FileOps, NewTest) {
Os::Tester tester;
tester.NewTest();
}
#endif


int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
Expand Down
Loading