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

Updated startIntTask in LinuxGpioDriver to take a stack size argument #2068

Merged

LeStarch merged 2 commits into nasa:devel from kubiak-jpl:dev/LinuxGpioDriver-StartIntTaskStack

Jun 8, 2023

Contributor

kubiak-jpl commented Jun 2, 2023


*Originating Project/Creator*	@kubiak-jpl
*Affected Component*	LinuxGpioDriver
*Affected Architectures(s)*	Linux
*Related Issue(s)*	N/A
*Has Unit Tests (y/n)*	No
*Builds Without Errors (y/n)*	Yes
*Unit Tests Pass (y/n)*	N/A
*Documentation Included (y/n)*	No

Change Description

Updates the startIntTask method call in LinuxGpioDriver to include a stack size argument for the new thread. This matches the behavior of the read thread call in LinuxUartDriver.

The stack size argument was inserted into the middle of the list of arguments to match LinuxUartDriver. However, existing code that uses both priority and CPU affinity arguments will be broken if directly updated. In practice I am unsure if this is too big of an issue as cpu affinity arguments will likely be invalid stack sizes.

Rationale

The application author is responsible for setting the stack size of the task, not the framework

Testing/Review Recommendations

Currently untested. Building LinuxGpioDriver still builds standalone and building this code in an internal project was successful


          Updated startIntTask in LinuxGpioDriver to take an optional stack siz…

107562e

…e argument

thomas-bc changed the base branch from master to devel

June 3, 2023 02:58

github-advanced-security bot found potential problems

View reviewed changes

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

                     Os::TaskString name;
                     name.format("GPINT_%s",this->getObjName()); // The task name can only be 16 chars including null
-                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, Os::Task::TASK_DEFAULT, cpuAffinity);
+                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, stackSize, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter priority has not been checked.

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

                     Os::TaskString name;
                     name.format("GPINT_%s",this->getObjName()); // The task name can only be 16 chars including null
-                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, Os::Task::TASK_DEFAULT, cpuAffinity);
+                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, stackSize, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter stackSize has not been checked.

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

                     Os::TaskString name;
                     name.format("GPINT_%s",this->getObjName()); // The task name can only be 16 chars including null
-                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, Os::Task::TASK_DEFAULT, cpuAffinity);
+                    Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, stackSize, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cpuAffinity has not been checked.

github-advanced-security bot found potential problems

View reviewed changes

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

@@ @@ -397,10 +397,10 @@ @@
                 }
                 Os::Task::TaskStatus LinuxGpioDriverComponentImpl ::
-                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE cpuAffinity) {
+                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE stackSize, NATIVE_UINT_TYPE cpuAffinity) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

priority uses the basic integral type unsigned int rather than a typedef with size and signedness.

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

@@ @@ -397,10 +397,10 @@ @@
                 }
                 Os::Task::TaskStatus LinuxGpioDriverComponentImpl ::
-                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE cpuAffinity) {
+                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE stackSize, NATIVE_UINT_TYPE cpuAffinity) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

stackSize uses the basic integral type unsigned int rather than a typedef with size and signedness.

Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp

@@ @@ -397,10 +397,10 @@ @@
                 }
                 Os::Task::TaskStatus LinuxGpioDriverComponentImpl ::
-                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE cpuAffinity) {
+                startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE stackSize, NATIVE_UINT_TYPE cpuAffinity) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

cpuAffinity uses the basic integral type unsigned int rather than a typedef with size and signedness.

LeStarch approved these changes

View reviewed changes

Collaborator

LeStarch left a comment

Looks good to me!

Collaborator

LeStarch commented Jun 3, 2023

Looks like the GPIO stubs file wasn't updated. @kubiak-jpl want to fix that? If not I'll get to it Monday.


          Updated LinuxGpioDriver Stubs for startIntTask update

b994928

LeStarch approved these changes

View reviewed changes

LeStarch approved these changes

View reviewed changes

LeStarch merged commit 5096ad4 into nasa:devel

23 checks passed

Boehm-Michael pushed a commit to Boehm-Michael/fprime that referenced this pull request


          Updated startIntTask in LinuxGpioDriver to take a stack size argument (…

85684b6

…nasa#2068)

* Updated startIntTask in LinuxGpioDriver to take an optional stack size argument

* Updated LinuxGpioDriver Stubs for startIntTask update

thomas-bc pushed a commit that referenced this pull request


          Updated startIntTask in LinuxGpioDriver to take a stack size argument (…

66b68eb

…#2068)

* Updated startIntTask in LinuxGpioDriver to take an optional stack size argument

* Updated LinuxGpioDriver Stubs for startIntTask update

thomas-bc added a commit that referenced this pull request


          Revert "Updated startIntTask in LinuxGpioDriver to take a stack size …

35b1964

…argument (#2068)"

This reverts commit 66b68eb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment