Skip to content

kernel: demote K_THREAD_STACK_BUFFER() to private #15195

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

Merged

Conversation

andrewboie
Copy link
Contributor

This macro is slated for complete removal, as it's not possible
on arches with an MPU stack guard to know the true buffer bounds
without also knowing the runtime state of its associated thread.

As removing this completely would be invasive to where we are
in the 1.14 release, demote to a private kernel Z_ API instead.
The current way that the macro is being used internally will
not cause any undue harm, we just don't want any external code
depending on it.

The final work to remove this (and overhaul stack specification in
general) will take place in 1.15 in the context of #14269

Fixes: #14766

Signed-off-by: Andrew Boie [email protected]

@andrewboie andrewboie added the bug The issue is a bug, or the PR is fixing a bug label Apr 4, 2019
@andrewboie andrewboie requested review from ioannisg and removed request for nategraff-sifive April 4, 2019 19:11
@andrewboie andrewboie force-pushed the remove-k-thread-stack-buffer branch from 8c50ffa to 70f6a47 Compare April 4, 2019 19:12
@zephyrbot
Copy link
Collaborator

zephyrbot commented Apr 4, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@andrewboie andrewboie force-pushed the remove-k-thread-stack-buffer branch from 70f6a47 to 4120d3d Compare April 4, 2019 22:18
@andrewboie andrewboie added this to the v1.14.0 milestone Apr 5, 2019
include/kernel.h Outdated
/* Stack Start - Identical to K_THREAD_STACK_BUFFER() on the stack
* object. Represents thread-writable stack area without any extras.
*/
/* Represents thread-writable stack area without any extras */
Copy link
Member

@ioannisg ioannisg Apr 5, 2019

Choose a reason for hiding this comment

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

The comment is more suitable for the documentation of the whole thread_stack_info struct
As is. it refers to the start variable.

Therefore, you could perhaps update it as "Stack start. - Represents the start address of the thread-writable stack area..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

Looks good to me . I left only a comment for documentation update

This macro is slated for complete removal, as it's not possible
on arches with an MPU stack guard to know the true buffer bounds
without also knowing the runtime state of its associated thread.

As removing this completely would be invasive to where we are
in the 1.14 release, demote to a private kernel Z_ API instead.
The current way that the macro is being used internally will
not cause any undue harm, we just don't want any external code
depending on it.

The final work to remove this (and overhaul stack specification in
general) will take place in 1.15 in the context of zephyrproject-rtos#14269

Fixes: zephyrproject-rtos#14766

Signed-off-by: Andrew Boie <[email protected]>
@andrewboie andrewboie force-pushed the remove-k-thread-stack-buffer branch from 4120d3d to df96ef7 Compare April 5, 2019 18:54
@andrewboie andrewboie requested a review from ioannisg April 5, 2019 18:54
@nashif nashif merged commit 4e5c093 into zephyrproject-rtos:master Apr 5, 2019
@andrewboie andrewboie deleted the remove-k-thread-stack-buffer branch April 24, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants