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_SockAddr_t alignment issue on some architectures #295

Closed
jphickey opened this issue Dec 3, 2019 · 0 comments · Fixed by #334
Closed

OS_SockAddr_t alignment issue on some architectures #295

jphickey opened this issue Dec 3, 2019 · 0 comments · Fixed by #334
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Dec 3, 2019

Describe the bug
On some CPU architectures that have strict alignment requirements, the OS socket address storage buffer triggers a warning/error about casts that increase alignment. For example:

os-impl-bsd-sockets.c:200:9: error: cast increases required alignment of target type [-Werror=cast-align]
    sa = (const struct sockaddr *)Addr->AddrData;

To Reproduce
Build on an architecture that has strict alignment requirements (e.g. SPARC, MIPS, etc)

Expected behavior
Should build cleanly, no warnings.

System observed on:

  • MIPS Linux (QEMU)

Additional context
Not likely to be a "real" alignment issue as this specific instance follows a uint32 value, so it will already have 32 bit alignment already. Adding a union wrapper will squelch the warning though.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

jphickey added a commit to jphickey/osal that referenced this issue Dec 3, 2019
Add a union wrapper for the abstract data field such that it will
be aligned for 32 bit integer values and/or pointers, whichever is
greater.

This should be sufficient for casting to the system "struct sockaddr"
@skliper skliper added the bug label Dec 11, 2019
@skliper skliper added this to the 5.1.0 milestone Dec 11, 2019
skliper added a commit that referenced this issue Dec 30, 2019
Fix #295, #298, #305, #307, #308,
    #313, #314, #316, #321, #323
Reviewed and approved at 2019-12-18 CCB
@skliper skliper closed this as completed in 25b9efd Jan 8, 2020
jphickey added a commit to jphickey/osal that referenced this issue Aug 10, 2022
Create a new background job to handle the maintenance tasks
that had been performed in the ES main task as part of the
CFE_ES_ScanAppTable() routine.

All app state changes, including those invoked by messages,
are now handled by this job.

This also slightly changes the semantics of CFE_ES_RunLoop and
CFE_ES_ExitApp.  Now, the CFE_ES_RunLoop routine no longer requires
a RunStatus buffer.  Instead, the only thing that matters is the
RunStatus value that is eventually passed to CFE_ES_ExitApp after
the shutdown is complete.  This should be mostly backward compatible,
as the recommended app pattern would pass the same value to
both functions.

This commit also fixes nasa#480, as the value passed to CFE_ES_ExitApp
will not override a request that was already pending.
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Fix nasa#295, Resolve app table scanning race conditions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants