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

Anway shmget syscall #310

Merged
merged 14 commits into from
Jul 22, 2024
Merged

Anway shmget syscall #310

merged 14 commits into from
Jul 22, 2024

Conversation

Anway-Agte
Copy link
Contributor

Description

This PR adds comments for the shmget_syscall which deals with shared memory mapping

Type of change

  • This change adds documentation

How Has This Been Tested?

  • Test A - ut_lind_test_shmget_syscall

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been added to a pull request and/or merged in other modules (native-client, lind-glibc, lind-project)

@Anway-Agte Anway-Agte requested a review from namanlalitnyu July 18, 2024 02:48
@Anway-Agte Anway-Agte force-pushed the anway-shmget-syscall branch from c63b33d to 1cc0132 Compare July 18, 2024 03:35
@rennergade rennergade requested a review from pranav-bhatt July 18, 2024 14:16
namanlalitnyu
namanlalitnyu reviewed Jul 18, 2024
View reviewed changes
Copy link
Contributor

@namanlalitnyu namanlalitnyu left a comment

Choose a reason for hiding this comment

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

Some minor comments.

@rennergade rennergade requested a review from ve1nard July 18, 2024 21:51
ve1nard
ve1nard reviewed Jul 18, 2024
View reviewed changes
Copy link
Contributor

@ve1nard ve1nard left a comment

Choose a reason for hiding this comment

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

great job, a couple of minor changes requested

namanlalitnyu
namanlalitnyu approved these changes Jul 18, 2024
View reviewed changes
Copy link
Contributor

@namanlalitnyu namanlalitnyu left a comment

Choose a reason for hiding this comment

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

LGTM!

ve1nard
ve1nard approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@ve1nard ve1nard left a comment

Choose a reason for hiding this comment

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

lgtm

@rennergade rennergade requested a review from yashaswi2000 July 22, 2024 01:11
yashaswi2000
yashaswi2000 approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@yashaswi2000 yashaswi2000 left a comment

Choose a reason for hiding this comment

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

LGTM

rennergade
rennergade requested changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@rennergade rennergade left a comment

Choose a reason for hiding this comment

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

Minor changes requested. But the bigger thing is you haven't done much actual error testing. I suggest adding a test that intentionally triggers errnos.

Also did you say there was a bug in this syscall before or am I misremembering?

@rennergade rennergade merged commit 0276e79 into develop Jul 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rennergade rennergade rennergade approved these changes

@yashaswi2000 yashaswi2000 yashaswi2000 approved these changes

@ve1nard ve1nard ve1nard approved these changes

@namanlalitnyu namanlalitnyu namanlalitnyu approved these changes

@pranav-bhatt pranav-bhatt Awaiting requested review from pranav-bhatt

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Anway-Agte @rennergade @yashaswi2000 @ve1nard @namanlalitnyu