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.

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

OS_IntAck placeholder resolution #370

Closed
skliper opened this issue Mar 15, 2020 · 8 comments
Closed

OS_IntAck placeholder resolution #370

skliper opened this issue Mar 15, 2020 · 8 comments

Comments

@skliper
Copy link
Contributor

skliper commented Mar 15, 2020

Is your feature request related to a problem? Please describe.
"Placeholder" isn't sufficient (prototype only).

Describe the solution you'd like
Either implement with a NOT_IMPLEMENTED error response or eliminate prototype (deprecate if it isn't appropriate). If implemented, needs a mechanism to test (functional/coverage/unit).

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 5.1.0 milestone Mar 23, 2020
@skliper
Copy link
Contributor Author

skliper commented Mar 23, 2020

If implemented would also need functional test.

Certification issue (no implementation at all I'd consider as not meeting API spec).

@jphickey
Copy link
Contributor

Recommend deprecating these functions, as a OSAL cannot provide a consistent cross-platform behavior for a function like this, and nothing currently uses it nor is there a compelling use case for it.

Do we even need to deprecate functions that were never actually implemented, or can we just remove them?

@skliper
Copy link
Contributor Author

skliper commented Mar 23, 2020

"these functions" - you mean just IntAck or some/all the interrupt related calls? Looks like a few are implemented in the various sample OSALs but not very consistent. @ejtimmon Any GSFC apps use the OS_Int* calls? @acudmore opinions?

Currently easier to deprecate for now, remove at next major release (I'm hoping for this summer).

@jphickey
Copy link
Contributor

I was thinking of all the interrupt-related routines (get/set mask, lock/unlock, etc) as well as the shared memory and FPU routines that were never fully realized. Would be nice to remove all of them.

For items which were only "placeholders"/declaration only and never had any actual definition, this could never have been used so I'd be in favor of just removing it to save us from having to ever revisit it again.

But of course for anything that did have a definition, this is where the agreed-upon deprecation procedure is needed.

Either way is fine by me, I'd just like to see these items cleaned up as they were never really useful.

@ejtimmon
Copy link

Memory Manager uses OS_IntLock() and OS_IntUnlock(). That's the only place any of the OS_Int* functions are used in the GSFC apps.

@jphickey
Copy link
Contributor

jphickey commented Mar 24, 2020

Memory Manager uses OS_IntLock() and OS_IntUnlock().

I'm guessing it does this with the intent of loading the mem block "atomically" with the hope of preventing another task from writing to it while this happens?

IntLock/Unlock has always been a no-op on POSIX, and furthermore even on platforms where it does something, it will not achieve that exclusivity effect on Multi-Core CPUs, as it only locks the interrupts on the core which calls it, and other cores continue to run anyway, interrupt or not.

Probably worth re-evaulating what MM is trying to achieve with the intlock... might be able to simply take it out with no loss of function.

@ejtimmon
Copy link

I've created an MM ticket to investigate this.

@skliper
Copy link
Contributor Author

skliper commented Mar 26, 2020

#396 deprecates the OS_Int calls

@skliper skliper closed this as completed Mar 26, 2020
@skliper skliper removed this from the 5.1.0 milestone May 13, 2020
jphickey added a commit to jphickey/osal that referenced this issue Aug 10, 2022
Simple replacement of CFE_MISSION_ES_CDS_MAX_NAME_LEN with
CFE_MISSION_ES_CDS_MAX_FULL_NAME_LEN, to differentiate it
from CFE_MISSION_ES_CDS_MAX_NAME_LENGTH - which is the CDS
name without the app name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants