-
Notifications
You must be signed in to change notification settings - Fork 221
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.
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
Fix #1279, rtems allow variable length queue msgs. #1280
Conversation
3d064ad
to
5718469
Compare
Im not sure how to fix the commit message. |
@thesamprice - it was the pull request title (lowercase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make the size verification optional? Just skip the check if passed in size is zero.
@skliper |
@thesamprice Copy, I should have looked at the shared code: osal/src/os/shared/src/osapi-queue.c Lines 174 to 181 in 6e6afb4
Looks like both POSIX and VxWorks implementations allow variable size, so I'm curious why the extra check is in RTEMS. Just concerned someone may be depending on this check... although if it's only in a subset of the implementations that's a pretty fragile dependency. osal/src/os/vxworks/src/os-impl-queues.c Lines 155 to 178 in 6e6afb4
Linux: osal/src/os/posix/src/os-impl-queues.c Lines 249 to 280 in 6e6afb4
|
Maybe instead of just deleting the check (and checking for success twice), it could be updated to match the pattern in POSX/VxWorks. Unless someone comes back with a reason why the check could be required for RTEMS I don't have any issues with making it consistent. |
Recalling change request based on further conversation
If someone is relying on this check in rtems, their code will break when going to vxworks / posix. I would recommend doing the following only if the check is needed.
This would keep the existing functionality common across all platforms. The functionality should be documented also. I use the queues to route messages to udp/tcp so this rtems check is breaking for me only on rtems. Ill go ahead and fork osal, and make the change so our system can work on rtems. |
Closing pull request, this will be replaced by the posix mimic version. |
Checklist (Please check before submitting)
NA as Gov employee? NA as removing code not adding?
Describe the contribution
A clear and concise description of what the contribution is.
Drops check on size != rtems_size to allow variable length queue messages.
Matches the posix queue implementation.
Testing performed
Steps taken to test the contribution:
Build steps '...'
Built and ran a non cfs system using posix.
built and ran the same cfs system using rtems, this fix allowed system to run.
Execution steps '...'
Ran on RTEMS on microblaze.
Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.
System(s) tested on
RTEMS / Microblaze
Additional context
Add any other context about the contribution here.
Third party code
N/A
Contributor Info - All information REQUIRED for consideration of pull request
NASA GSFC