Skip to content

Fixed IPC_PRIVATE handling for SysV Shm.#1192

Merged
karya0 merged 1 commit intomainfrom
dev/karya0/shm_ipc_private
Mar 23, 2025
Merged

Fixed IPC_PRIVATE handling for SysV Shm.#1192
karya0 merged 1 commit intomainfrom
dev/karya0/shm_ipc_private

Conversation

@karya0
Copy link
Copy Markdown
Member

@karya0 karya0 commented Mar 2, 2025 •
edited by gc00
Loading

IPC_PRIVATE can [now] be called multiple times and it will in turn create new handlers every time.

Also updated sysv-shm1 test to create shm segments with IPC_PRIVATE key.

Thanks @gkliu for reporting the bug and creating an initial PR (#1191) with the fix.

@karya0 karya0 requested a review from gc00 March 2, 2025 05:03
@karya0 karya0 mentioned this pull request Mar 2, 2025
Closed
@karya0 karya0 force-pushed the dev/karya0/shm_ipc_private branch from 4190f85 to 3e8a31a Compare March 2, 2025 06:28
gc00
gc00 requested changes Mar 6, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@gc00 gc00 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@karya0 (and @gkliu), Thanks for this PR. It's an important fix.

@karya0 , I have one suggestion: Since the optional argument in SharedData::getRealIPCId() is always 'true', why not remove the optional argument? But if you have a reason for keeping the optional arg, let me know, and I'll still approve it.

@karya0 karya0 force-pushed the dev/karya0/shm_ipc_private branch from 3e8a31a to b235ebf Compare March 8, 2025 18:11
@karya0 karya0 force-pushed the dev/karya0/shm_ipc_private branch from b235ebf to 6e7fcf3 Compare March 9, 2025 10:19
@karya0 karya0 requested a review from gc00 March 9, 2025 10:20
@gc00
Copy link
Copy Markdown
Contributor

gc00 commented Mar 11, 2025

I'm not sure if I'm the one who should be replying after an update. Just in case, this is still the only concern that I have:

@karya0 , I have one suggestion: Since the optional argument in SharedData::getRealIPCId() is always 'true', why not remove the optional argument? But if you have a reason for keeping the optional arg, let me know, and I'll still approve it.

@karya0
Copy link
Copy Markdown
Member Author

karya0 commented Mar 11, 2025

I'm not sure if I'm the one who should be replying after an update. Just in case, this is still the only concern that I have:

@karya0 , I have one suggestion: Since the optional argument in SharedData::getRealIPCId() is always 'true', why not remove the optional argument? But if you have a reason for keeping the optional arg, let me know, and I'll still approve it.

@gc00 : I already fixed a typo and added a response to your comment about the optional argument. Please take a look and approve if it looks good.

gc00
gc00 approved these changes Mar 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@gc00 gc00 left a comment

Choose a reason for hiding this comment

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

Thanks. I have no mroe comments.
LGTM.

@karya0 karya0 merged commit 3ab9450 into main Mar 23, 2025
1 check passed
@karya0 karya0 deleted the dev/karya0/shm_ipc_private branch April 30, 2025 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gc00 gc00 gc00 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@karya0 @gc00