Skip to content

Commit b9a8611

Browse files
zimri-leisherzimri-leisher
authored
Merge branch 'devel' into 5063-fpy-seq-dir
2 parents 32c93b3 + 060f0e1 commit b9a8611

16 files changed

Lines changed: 900 additions & 70 deletions

File tree

‎Os/File.cpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
// \brief common function implementation for Os::File
44
// ======================================================================
55
#include <Fw/Types/Assert.hpp>
6+
#include <Fw/Types/StringUtils.hpp>
67
#include <Os/File.hpp>
78
#include <algorithm>
9+
#include <config/FppConstantsAc.hpp>
810

911
extern "C" {
1012
#include <Utils/Hash/libcrc/lib_crc.h> // borrow CRC
@@ -48,8 +50,22 @@ File::Status File::open(const CHAR* filepath, File::Mode requested_mode) {
4850
}
4951

5052
File::Status File::open(const CHAR* filepath, File::Mode requested_mode, File::OverwriteType overwrite) {
53+
FW_ASSERT(nullptr != filepath);
54+
return this->open(filepath, static_cast<FwSizeType>(FileNameStringSize + 1), requested_mode, overwrite);
55+
}
56+
57+
File::Status File::open(const CHAR* filepath, FwSizeType length, File::Mode requested_mode) {
58+
return this->open(filepath, length, requested_mode, OverwriteType::NO_OVERWRITE);
59+
}
60+
61+
File::Status File::open(const CHAR* filepath,
62+
FwSizeType length,
63+
File::Mode requested_mode,
64+
File::OverwriteType overwrite) {
5165
FW_ASSERT(&this->m_delegate == reinterpret_cast<FileInterface*>(&this->m_handle_storage[0]));
5266
FW_ASSERT(nullptr != filepath);
67+
const FwSizeType string_len = static_cast<FwSizeType>(Fw::StringUtils::string_length(filepath, length));
68+
FW_ASSERT(string_len < length, static_cast<FwAssertArgType>(string_len), static_cast<FwAssertArgType>(length));
5369
FW_ASSERT(File::Mode::OPEN_NO_MODE < requested_mode && File::Mode::MAX_OPEN_MODE > requested_mode);
5470
FW_ASSERT((0 <= this->m_mode) && (this->m_mode < Mode::MAX_OPEN_MODE));
5571
FW_ASSERT((0 <= overwrite) && (overwrite < OverwriteType::MAX_OVERWRITE_TYPE));
@@ -68,6 +84,15 @@ File::Status File::open(const CHAR* filepath, File::Mode requested_mode, File::O
6884
return status;
6985
}
7086

87+
File::Status File::open(const Fw::ConstStringBase& path, File::Mode requested_mode) {
88+
return this->open(path.toChar(), static_cast<FwSizeType>(path.getCapacity()), requested_mode,
89+
OverwriteType::NO_OVERWRITE);
90+
}
91+
92+
File::Status File::open(const Fw::ConstStringBase& path, File::Mode requested_mode, File::OverwriteType overwrite) {
93+
return this->open(path.toChar(), static_cast<FwSizeType>(path.getCapacity()), requested_mode, overwrite);
94+
}
95+
7196
void File::close() {
7297
FW_ASSERT(&this->m_delegate == reinterpret_cast<FileInterface*>(&this->m_handle_storage[0]));
7398
FW_ASSERT(this->m_mode < Mode::MAX_OPEN_MODE);

‎Os/File.hpp‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define Os_File_hpp_
77

88
#include <Fw/FPrimeBasicTypes.hpp>
9+
#include <Fw/Types/ConstStringBase.hpp>
910
#include <Os/Os.hpp>
1011

1112
// Forward declaration for UTs
@@ -264,6 +265,51 @@ class File final : public FileInterface {
264265
//!
265266
Os::FileInterface::Status open(const char* path, Mode mode);
266267

268+
//! \brief open file with supplied path, bounded length, and mode
269+
//!
270+
//! Open the file passed in with the given mode. The path length is bounded by `length`.
271+
//! Opening files with `OPEN_CREATE` mode will not clobber existing files. Use the overload
272+
//! accepting `OverwriteType` to set overwrite flag and clobber existing files.
273+
//!
274+
//! It is invalid to send `nullptr` as the path.
275+
//! It is invalid to supply `mode` as a non-enumerated value.
276+
//! It is invalid for the path to not be null-terminated within `length` characters.
277+
//!
278+
//! \param path: c-string of path to open
279+
//! \param length: bound on the path buffer size
280+
//! \param mode: file operation mode
281+
//! \return: status of the open
282+
//!
283+
Os::FileInterface::Status open(const char* path, FwSizeType length, Mode mode);
284+
285+
//! \brief open file with supplied string path and mode
286+
//!
287+
//! Open the file passed in with the given mode. Opening files with `OPEN_CREATE` mode will not clobber existing
288+
//! files. Use the overload accepting `OverwriteType` to set overwrite flag and clobber existing files.
289+
//!
290+
//! It is invalid to supply `mode` as a non-enumerated value.
291+
//!
292+
//! \param path: ConstStringBase reference of path to open
293+
//! \param mode: file operation mode
294+
//! \return: status of the open
295+
//!
296+
Os::FileInterface::Status open(const Fw::ConstStringBase& path, Mode mode);
297+
298+
//! \brief open file with supplied string path, mode, and overwrite type
299+
//!
300+
//! Open the file passed in with the given mode. If overwrite is set to OVERWRITE, then opening files in
301+
//! OPEN_CREATE mode will clobber existing files. Set overwrite to NO_OVERWRITE to preserve existing files.
302+
//!
303+
//! It is invalid to supply `mode` as a non-enumerated value.
304+
//! It is invalid to supply `overwrite` as a non-enumerated value.
305+
//!
306+
//! \param path: ConstStringBase reference of path to open
307+
//! \param mode: file operation mode
308+
//! \param overwrite: overwrite existing file on create
309+
//! \return: status of the open
310+
//!
311+
Os::FileInterface::Status open(const Fw::ConstStringBase& path, Mode mode, OverwriteType overwrite);
312+
267313
//! \brief read data from this file into supplied buffer bounded by size
268314
//!
269315
//! Read data from this file up to the `size` and store it in `buffer`. This version will
@@ -321,6 +367,26 @@ class File final : public FileInterface {
321367
//!
322368
Os::FileInterface::Status open(const char* path, Mode mode, OverwriteType overwrite) override;
323369

370+
//! \brief open file with supplied path, bounded length, mode, and overwrite type
371+
//!
372+
//! Open the file passed in with the given mode. The path length is bounded by `length`.
373+
//! If overwrite is set to OVERWRITE, then opening files in OPEN_CREATE mode will clobber
374+
//! existing files. Set overwrite to NO_OVERWRITE to preserve existing files. This is the
375+
//! core open implementation to which all other open overloads delegate.
376+
//!
377+
//! It is invalid to send `nullptr` as the path.
378+
//! It is invalid to supply `mode` as a non-enumerated value.
379+
//! It is invalid to supply `overwrite` as a non-enumerated value.
380+
//! It is invalid for the path to not be null-terminated within `length` characters.
381+
//!
382+
//! \param path: c-string of path to open
383+
//! \param length: bound on the path buffer size
384+
//! \param mode: file operation mode
385+
//! \param overwrite: overwrite existing file on create
386+
//! \return: status of the open
387+
//!
388+
Os::FileInterface::Status open(const char* path, FwSizeType length, Mode mode, OverwriteType overwrite);
389+
324390
//! \brief close the file, if not opened then do nothing
325391
//!
326392
//! Closes the file, if open. Otherwise this function does nothing. Delegates to the chosen implementation's

‎Os/Posix/test/ut/PosixFileTests.cpp‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// ======================================================================
55
#include <gtest/gtest.h>
66
#include <unistd.h>
7+
#include <config/FppConstantsAc.hpp>
78
#include <csignal>
89
#include <cstdio>
910
#include <list>
@@ -17,7 +18,7 @@ namespace FileTest {
1718

1819
std::vector<std::shared_ptr<const std::string> > FILES;
1920

20-
static const U32 MAX_FILES = 100;
21+
static const U32 MAX_FILES = 500;
2122
static const char BASE_PATH[] = "/tmp/fprime";
2223
static const char TEST_FILE[] = "fprime-os-file-test";
2324
//! Check if we can use the file. F_OK file exists, R_OK, W_OK are read and write.
@@ -33,21 +34,26 @@ bool check_permissions(const char* path, int permission) {
3334
//!
3435
std::shared_ptr<std::string> get_test_filename(bool random) {
3536
const char* filename = TEST_FILE;
36-
char full_buffer[_POSIX_PATH_MAX];
37-
char buffer[_POSIX_PATH_MAX - sizeof(BASE_PATH)];
37+
// FileNameStringSize is the max string length; +1 for null terminator
38+
char full_buffer[FileNameStringSize + 1];
39+
// Cap random part so full path (BASE_PATH + '/' + random + '\0') fits within FileNameStringSize.
40+
// sizeof(BASE_PATH) accounts for strlen(BASE_PATH) + null terminator, which equals the prefix
41+
// length (strlen(BASE_PATH) + '/') by coincidence. Subtract 1 to account for the null terminator.
42+
static const size_t MAX_RANDOM_LEN = FileNameStringSize - sizeof(BASE_PATH) - 1;
43+
char buffer[MAX_RANDOM_LEN + 1];
3844
// When random, select random characters
3945
if (random) {
4046
filename = buffer;
4147
size_t i = 0;
42-
for (i = 0; i < STest::Pick::lowerUpper(2, (sizeof buffer) - 1); i++) {
48+
for (i = 0; i < STest::Pick::lowerUpper(2, MAX_RANDOM_LEN); i++) {
4349
char selected_character = static_cast<char>(STest::Pick::lowerUpper(48, 126));
4450
selected_character =
4551
(selected_character == '/') ? static_cast<char>(selected_character + 1) : selected_character;
4652
buffer[i] = selected_character;
4753
}
4854
buffer[i] = 0; // Terminate random string
4955
}
50-
(void)snprintf(full_buffer, _POSIX_PATH_MAX, "%s/%s", BASE_PATH, filename);
56+
(void)snprintf(full_buffer, sizeof(full_buffer), "%s/%s", BASE_PATH, filename);
5157
// Create a shared pointer wrapping our filename buffer
5258
std::shared_ptr<std::string> pointer(new std::string(full_buffer), std::default_delete<std::string>());
5359
return pointer;

‎Os/test/ut/file/CommonTests.cpp‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ TEST_F(Functionality, CopyConstructor) {
6666
close_rule.apply(*tester);
6767
}
6868

69+
// Ensure that open works via the bounded char* overload
70+
TEST_F(Functionality, OpenWithCreationBounded) {
71+
Os::Test::FileTest::Tester::OpenFileCreateBounded rule(false);
72+
rule.apply(*tester);
73+
}
74+
75+
// Ensure that open works via the ConstStringBase overload
76+
TEST_F(Functionality, OpenWithCreationString) {
77+
Os::Test::FileTest::Tester::OpenFileCreateString rule(false);
78+
rule.apply(*tester);
79+
}
80+
81+
// Ensure that open asserts on unterminated path within bounds
82+
TEST_F(InvalidArguments, OpenBoundedPathUnterminated) {
83+
Os::Test::FileTest::Tester::OpenIllegalBoundedPath rule;
84+
rule.apply(*tester);
85+
}
86+
6987
// Ensure that open on existence works
7088
TEST_F(FunctionalIO, OpenWithCreationExists) {
7189
Os::Test::FileTest::Tester::OpenFileCreate open_rule(false);
@@ -310,6 +328,8 @@ TEST_F(Functionality, RandomizedInterfaceTesting) {
310328
Os::Test::FileTest::Tester::WriteIllegalBuffer write_illegal_buffer;
311329
Os::Test::FileTest::Tester::IncrementalCrcInvalidModes incremental_invalid_mode_rule;
312330
Os::Test::FileTest::Tester::FullCrcInvalidModes full_invalid_mode_rule;
331+
Os::Test::FileTest::Tester::OpenFileCreateBounded open_file_create_bounded_rule(true);
332+
Os::Test::FileTest::Tester::OpenFileCreateString open_file_create_string_rule(true);
313333

314334
// Place these rules into a list of rules
315335
STest::Rule<Os::Test::FileTest::Tester>* rules[] = {&open_file_create_overwrite_rule,
@@ -328,7 +348,9 @@ TEST_F(Functionality, RandomizedInterfaceTesting) {
328348
&read_illegal_buffer,
329349
&write_illegal_buffer,
330350
&incremental_invalid_mode_rule,
331-
&full_invalid_mode_rule};
351+
&full_invalid_mode_rule,
352+
&open_file_create_bounded_rule,
353+
&open_file_create_string_rule};
332354

333355
// Take the rules and place them into a random scenario
334356
STest::RandomScenario<Os::Test::FileTest::Tester> random("Random Rules", rules, FW_NUM_ARRAY_ELEMENTS(rules));
@@ -368,6 +390,8 @@ TEST_F(FunctionalIO, RandomizedTesting) {
368390
Os::Test::FileTest::Tester::WriteInvalidModes write_invalid_modes_rule;
369391
Os::Test::FileTest::Tester::IncrementalCrcInvalidModes incremental_invalid_mode_rule;
370392
Os::Test::FileTest::Tester::FullCrcInvalidModes full_invalid_mode_rule;
393+
Os::Test::FileTest::Tester::OpenFileCreateBounded open_file_create_bounded_rule(true);
394+
Os::Test::FileTest::Tester::OpenFileCreateString open_file_create_string_rule(true);
371395

372396
// Place these rules into a list of rules
373397
STest::Rule<Os::Test::FileTest::Tester>* rules[] = {&open_file_create_rule,
@@ -393,7 +417,9 @@ TEST_F(FunctionalIO, RandomizedTesting) {
393417
&read_invalid_modes_rule,
394418
&write_invalid_modes_rule,
395419
&incremental_invalid_mode_rule,
396-
&full_invalid_mode_rule};
420+
&full_invalid_mode_rule,
421+
&open_file_create_bounded_rule,
422+
&open_file_create_string_rule};
397423

398424
// Take the rules and place them into a random scenario
399425
STest::RandomScenario<Os::Test::FileTest::Tester> random("Random Rules", rules, FW_NUM_ARRAY_ELEMENTS(rules));

0 commit comments

Comments
 (0)