Skip to content

Dup/dup2 syscall tests and comments#285

Merged
rennergade merged 11 commits intodevelopfrom
dup_tests
Jul 9, 2024
Merged

Dup/dup2 syscall tests and comments#285
rennergade merged 11 commits intodevelopfrom
dup_tests

Conversation

@rupeshkoushik07
Copy link
Copy Markdown
Member

@rupeshkoushik07 rupeshkoushik07 commented Jun 28, 2024 •
edited
Loading

Description

Adds dup/dup2 syscall comments and tests.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Test A - lind_project/tests/test_cases/test_a.c
  • Test B - lind_project/tests/test_cases/test_b.c

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)

@JustinCappos
Copy link
Copy Markdown
Member

JustinCappos commented Jun 28, 2024

Please pick one new student to review your PR first. After addressing those issues, please ping me and/or Nick. (We're trying not to overwhelm people with asks to review.)

@rupeshkoushik07
Copy link
Copy Markdown
Member Author

rupeshkoushik07 commented Jun 29, 2024

@davidge20 requesting your review

@rupeshkoushik07 rupeshkoushik07 requested a review from ve1nard July 1, 2024 16:27
@rennergade
Copy link
Copy Markdown
Contributor

rennergade commented Jul 1, 2024

@namanlalitnyu can you review this one? David won't be able to get to it for awhile. Thanks!

davidge20
davidge20 reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
Comment thread src/tests/fs_tests.rs
Comment thread src/safeposix/syscalls/fs_calls.rs
@davidge20
Copy link
Copy Markdown
Contributor

davidge20 commented Jul 2, 2024

@namanlalitnyu can you review this one? David won't be able to get to it for awhile. Thanks!

Had some time so hopefully I gave a helpful review :)

namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs
namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs
namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
namanlalitnyu
namanlalitnyu reviewed Jul 2, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Have some minor comments, rest looks good.

namanlalitnyu
namanlalitnyu approved these changes Jul 3, 2024
View reviewed changes
Copy link
Copy Markdown
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!

@rennergade
Copy link
Copy Markdown
Contributor

rennergade commented Jul 3, 2024

Looks like you need to fix a merge conflict. @yashaswi2000 can you review this today?

@yashaswi2000 yashaswi2000 mentioned this pull request Jul 3, 2024
Open
@rupeshkoushik07 rupeshkoushik07 requested a review from davidge20 July 7, 2024 07:17
yashaswi2000
yashaswi2000 approved these changes Jul 8, 2024
View reviewed changes
Copy link
Copy Markdown
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 reviewed Jul 8, 2024
View reviewed changes
Comment thread src/safeposix/syscalls/fs_calls.rs Outdated
@rennergade
Copy link
Copy Markdown
Contributor

rennergade commented Jul 8, 2024

If you fix the imports thing I think this is ready to go!

rennergade
rennergade approved these changes Jul 9, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Approved

@rennergade rennergade merged commit f09995f into develop Jul 9, 2024
Anway-Agte pushed a commit that referenced this pull request Jul 15, 2024
@rupeshkoushik07
Dup/dup2 syscall tests and comments (#285)
5223a15 
* add tests

* update dup2 max

* change error

* update the comments and fromatting

* update files/streams

* add test

* remove

* remove

* remove import
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

@JustinCappos JustinCappos Awaiting requested review from JustinCappos

@ve1nard ve1nard Awaiting requested review from ve1nard

@davidge20 davidge20 Awaiting requested review from davidge20

+1 more reviewer

@namanlalitnyu namanlalitnyu namanlalitnyu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rupeshkoushik07 @JustinCappos @rennergade @davidge20 @yashaswi2000 @namanlalitnyu
⚡