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.

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

BUGFIX: change unsuitable atomic_int to atomic_flag #472

Merged
merged 2 commits into from
Mar 14, 2023
Merged

BUGFIX: change unsuitable atomic_int to atomic_flag #472

merged 2 commits into from
Mar 14, 2023

Conversation

mzyx-hnu
Copy link
Contributor

@mzyx-hnu mzyx-hnu commented Mar 1, 2023

This patch tries to fix issue#471

@mzyx-hnu mzyx-hnu mentioned this pull request Mar 1, 2023
Closed
@arnopo arnopo linked an issue Mar 3, 2023 that may be closed by this pull request
atomic_flag func error when compile with clang #471
Closed
arnopo
arnopo requested changes Mar 3, 2023
View reviewed changes
Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

The fix seems valid to me. I wonder if we need to have a atomic_flag_test_and_set on init, but that seems inline with the initial value set by atomic_init...

One cosmetic remark:
Please, split in 2 commits: one for the lib one` for the app. have look to existing commit to see commit subjet prefix to use

apps/examples/linux_rpc_demo/linux_rpc_demo.c Show resolved Hide resolved
lib/proxy/rpmsg_retarget.c Show resolved Hide resolved
@arnopo arnopo requested a review from edmooring March 3, 2023 13:42
edmooring
edmooring reviewed Mar 4, 2023
View reviewed changes
Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Not much to say here. I think this will be good to go with @arnopo comments addressed.

@mzyx-hnu
apps:Fix atomic_flag error for clang compilation
124b01e 
Change atomic_int to atomic_flag to solve the error
reported when compiling with clang.

Signed-off-by: Yunfei Li <liyunfei33@huawei.com>
@mzyx-hnu
lib:Fix atomic_flag error for clang compilation
e49995a 
Change atomic_int to atomic_flag to solve the error
reported when compiling with clang.

Signed-off-by: Yunfei Li <liyunfei33@huawei.com>
@mzyx-hnu
Copy link
Contributor Author

mzyx-hnu commented Mar 6, 2023

The fix seems valid to me. I wonder if we need to have a atomic_flag_test_and_set on init, but that seems inline with the initial value set by atomic_init...

One cosmetic remark: Please, split in 2 commits: one for the lib one` for the app. have look to existing commit to see commit subjet prefix to use

updated as the above comments

@mzyx-hnu mzyx-hnu requested a review from arnopo March 6, 2023 08:29
@arnopo arnopo requested a review from edmooring March 7, 2023 10:35
edmooring
edmooring approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Good to go.

@arnopo arnopo added this to the Release V2023.04 milestone Mar 14, 2023
@arnopo arnopo merged commit 1b97548 into OpenAMP:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edmooring edmooring edmooring approved these changes

@arnopo arnopo arnopo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release V2023.04
Development

Successfully merging this pull request may close these issues.

atomic_flag func error when compile with clang
3 participants
@mzyx-hnu @edmooring @arnopo