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

Jump to bottom

Os file vxworks #3009

Merged
merged 7 commits into from
Nov 7, 2024
Merged

Os file vxworks #3009

merged 7 commits into from
Nov 7, 2024

Conversation

kevin-f-ortega
Copy link
Collaborator

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Adding macros to include/exclude code when compiling for VxWorks.

Rationale

This allows VxWorks to build with the Posix file implementation.

Testing/Review Recommendations

I ran a simple hello world program in the VxSim. The hello world program opened a file and wrote some bytes to the file.

Future Work

Note any additional work that will be done relating to this issue.

@kevin-f-ortega
Copy link
Collaborator Author

NOTE: I used F Prime's .clang-format to format the files I modified.

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 6, 2024
View reviewed changes
Os/Posix/File.cpp Fixed Show fixed Hide fixed
Os/Posix/File.cpp Fixed Show fixed Hide fixed
Os/Posix/File.cpp
}

PosixFile::Status PosixFile::open(const char* filepath, PosixFile::Mode requested_mode, PosixFile::OverwriteType overwrite) {
PosixFile::Status PosixFile::open(const char* filepath,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/File.cpp
@@ -95,7 +111,7 @@
}
}

PosixFile::Status PosixFile::size(FwSignedSizeType& size_result) {
PosixFile::Status PosixFile::size(FwSignedSizeType& size_result) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/File.cpp
@@ -118,7 +134,7 @@
return status;
}

PosixFile::Status PosixFile::position(FwSignedSizeType &position_result) {
PosixFile::Status PosixFile::position(FwSignedSizeType& position_result) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/File.cpp
@@ -198,18 +216,18 @@
return status;
}

PosixFile::Status PosixFile::read(U8* buffer, FwSignedSizeType &size, PosixFile::WaitType wait) {
PosixFile::Status PosixFile::read(U8* buffer, FwSignedSizeType& size, PosixFile::WaitType wait) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/File.cpp
@@ -234,15 +252,19 @@
return status;
}

PosixFile::Status PosixFile::write(const U8* buffer, FwSignedSizeType &size, PosixFile::WaitType wait) {
PosixFile::Status PosixFile::write(const U8* buffer, FwSignedSizeType& size, PosixFile::WaitType wait) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/FileSystem.cpp
@@ -6,7 +6,9 @@
#include "Os/Posix/error.hpp"

#include <dirent.h>
#ifndef TGT_OS_TYPE_VXWORKS

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
Os/Posix/FileSystem.cpp
@@ -58,11 +59,19 @@
return status;
}

PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path, FwSizeType& totalBytes, FwSizeType& freeBytes) {
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/FileSystem.cpp
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path,
FwSizeType& totalBytes,
FwSizeType& freeBytes) {
#ifdef TGT_OS_TYPE_VXWORKS

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 6, 2024
View reviewed changes
Os/Posix/File.cpp
}

PosixFile::Status PosixFile::open(const char* filepath, PosixFile::Mode requested_mode, PosixFile::OverwriteType overwrite) {
PosixFile::Status PosixFile::open(const char* filepath,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

filepath uses the basic integral type char rather than a typedef with size and signedness.
Os/Posix/File.cpp
@@ -95,7 +111,7 @@
}
}

PosixFile::Status PosixFile::size(FwSignedSizeType& size_result) {
PosixFile::Status PosixFile::size(FwSignedSizeType& size_result) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

size_result uses the basic integral type signed long rather than a typedef with size and signedness.
Os/Posix/File.cpp
@@ -118,7 +134,7 @@
return status;
}

PosixFile::Status PosixFile::position(FwSignedSizeType &position_result) {
PosixFile::Status PosixFile::position(FwSignedSizeType& position_result) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

position_result uses the basic integral type signed long rather than a typedef with size and signedness.
Os/Posix/File.cpp
@@ -179,7 +196,8 @@

PosixFile::Status PosixFile::seek(FwSignedSizeType offset, PosixFile::SeekType seekType) {
Status status = OP_OK;
off_t actual = ::lseek(this->m_handle.m_file_descriptor, offset, (seekType == SeekType::ABSOLUTE) ? SEEK_SET : SEEK_CUR);
off_t actual =

Check notice

Code scanning / CodeQL

Use of basic integral type Note

actual uses the basic integral type long rather than a typedef with size and signedness.
Os/Posix/File.cpp
@@ -198,18 +216,18 @@
return status;
}

PosixFile::Status PosixFile::read(U8* buffer, FwSignedSizeType &size, PosixFile::WaitType wait) {
PosixFile::Status PosixFile::read(U8* buffer, FwSignedSizeType& size, PosixFile::WaitType wait) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

size uses the basic integral type signed long rather than a typedef with size and signedness.
Os/Posix/File.cpp

for (FwSignedSizeType i = 0; i < maximum && accumulated < size; i++) {
// char* for some posix implementations
ssize_t write_size = ::write(this->m_handle.m_file_descriptor, reinterpret_cast<const CHAR*>(&buffer[accumulated]), static_cast<size_t>(size - accumulated));
ssize_t write_size =

Check notice

Code scanning / CodeQL

Use of basic integral type Note

write_size uses the basic integral type long rather than a typedef with size and signedness.
Os/Posix/File.cpp
@@ -258,11 +280,11 @@
size = accumulated;
// When waiting, sync to disk
if (wait) {
PlatformIntType fsync_return = ::fsync(this->m_handle.m_file_descriptor);
if (PosixFileHandle::ERROR_RETURN_VALUE == fsync_return) {
PlatformIntType fsync_return = ::fsync(this->m_handle.m_file_descriptor);

Check notice

Code scanning / CodeQL

Use of basic integral type Note

fsync_return uses the basic integral type int rather than a typedef with size and signedness.
Os/Posix/FileSystem.cpp
@@ -58,11 +59,19 @@
return status;
}

PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path, FwSizeType& totalBytes, FwSizeType& freeBytes) {
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

path uses the basic integral type char rather than a typedef with size and signedness.
Os/Posix/FileSystem.cpp
@@ -58,11 +59,19 @@
return status;
}

PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path, FwSizeType& totalBytes, FwSizeType& freeBytes) {
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path,
FwSizeType& totalBytes,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

totalBytes uses the basic integral type unsigned long rather than a typedef with size and signedness.
Os/Posix/FileSystem.cpp
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path, FwSizeType& totalBytes, FwSizeType& freeBytes) {
PosixFileSystem::Status PosixFileSystem::_getFreeSpace(const char* path,
FwSizeType& totalBytes,
FwSizeType& freeBytes) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

freeBytes uses the basic integral type unsigned long rather than a typedef with size and signedness.
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 6, 2024
View reviewed changes
Os/Posix/File.cpp
@@ -179,7 +196,8 @@

PosixFile::Status PosixFile::seek(FwSignedSizeType offset, PosixFile::SeekType seekType) {
Status status = OP_OK;
off_t actual = ::lseek(this->m_handle.m_file_descriptor, offset, (seekType == SeekType::ABSOLUTE) ? SEEK_SET : SEEK_CUR);
off_t actual =
::lseek(this->m_handle.m_file_descriptor, offset, (seekType == SeekType::ABSOLUTE) ? SEEK_SET : SEEK_CUR);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter offset has not been checked.
Os/Posix/File.cpp
this->m_handle.m_file_descriptor,
reinterpret_cast<CHAR*>(&buffer[accumulated]),
static_cast<size_t>(size - accumulated));
ssize_t read_size = ::read(this->m_handle.m_file_descriptor, reinterpret_cast<CHAR*>(&buffer[accumulated]),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
Os/Posix/File.cpp

for (FwSignedSizeType i = 0; i < maximum && accumulated < size; i++) {
// char* for some posix implementations
ssize_t write_size = ::write(this->m_handle.m_file_descriptor, reinterpret_cast<const CHAR*>(&buffer[accumulated]), static_cast<size_t>(size - accumulated));
ssize_t write_size =
::write(this->m_handle.m_file_descriptor, reinterpret_cast<const CHAR*>(&buffer[accumulated]),

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter buffer has not been checked.
LeStarch
LeStarch reviewed Nov 6, 2024
View reviewed changes
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

This is currently a runtime-check that asserts when running. Would it be better to do a build-time check? This could be done by conditionally setting the constructors to =delete in the header in the VxWorks build.

Rather than crash the FSW, this would fail to link if that code was ever used. Thoughts?

@kevin-f-ortega
Adding pound-define code for VxWorks otherwise this does not build wh…
da5de71 
…en building VxWorks using Posix. Better to do a pound-if here than copying the entire file and deleting a line in the vxworks repo
@kevin-f-ortega
Adding comment
3d66b1e
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 7, 2024
View reviewed changes
Os/Posix/File.cpp
@@ -43,19 +45,25 @@
"Minimum value of FwSizeType larger than the minimum value of ssize_t. Configure a larger type.");

//!\brief default copy constructor
#ifndef TGT_OS_TYPE_VXWORKS

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
Os/Posix/File.cpp

#ifndef TGT_OS_TYPE_VXWORKS

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
@LeStarch LeStarch merged commit 3c2ac53 into devel Nov 7, 2024
36 checks passed
@thomas-bc thomas-bc deleted the os-file-vxworks branch November 8, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeStarch LeStarch LeStarch approved these changes

@thomas-bc thomas-bc Awaiting requested review from thomas-bc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kevin-f-ortega @LeStarch