chmod_syscall() updates

[ve1nard]

Description

Fixes # (issue)

The following changes include more elaborate comments and new unit tests for chmod_syscall and fchmod_syscall.

Type of change

• More detailed comments for chmod_syscall and fchmod_syscall
• Shifted checking the validity of mode bits from chmod_syscall() to the _chmod_helper() helper function since it used by both chmod_syscall() and fchmod_syscall()
• Removed the unnecessary check for file type flags that allowed the file type to be changed
• New unit tests for chmod_syscall and fchmod_syscall

How Has This Been Tested?

To run the tests, we need to run cargo test --lib command inside the safeposix-rust directory.

All the tests are present under this directory: lind_project/src/safeposix-rust/src/tests/fs_tests.rs

• Test A - lut_lind_fs_chmod_valid_args()
• Test B - ut_lind_fs_chmod_invalid_args()
• Test C - ut_lind_fs_fchmod()

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]

Let's ping one person at a time on reviews. Please pick a peer who is also getting started. Then get Yash or Rupesh to review after that. Then me or Nick for the final pass. Over time we will promote people up so fewer reviews are needed...

…

On Sat, Jun 22, 2024 at 5:34 PM ve1nard ***@***.***> wrote: @ve1nard <https://github.com/ve1nard> requested your review on: #275 <#275> chmod_syscall() updates. — Reply to this email directly, view it on GitHub <#275 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGROD72XBPYTVP4IB3N2CDZIXUWDAVCNFSM6AAAAABJXXZO5GVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJTGI2TCOBQGA2DONA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[qianxichen233]

Yeah, but I did not see anywhere you filtered file type bits out, so user can still pass file type bits to change the file type using chmod ve1nard ***@***.***>于2024年6月23日 周日21:03写道：

…

***@***.**** commented on this pull request. ------------------------------ In src/safeposix/syscalls/fs_calls.rs <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Lind-2DProject_safeposix-2Drust_pull_275-23discussion-5Fr1650232706&d=DwMFaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=mwCephJ0TxjvnvPU0MlAEw&m=VO2tFj5V3hpn5LJQbM6WNXis7Pl-UofHAelqK455muSnPR1mNM5GAnXxswYR9Rrw&s=wMeuY5JxmBODJ794hjmF0UpknbtbGDWrW9MyILhoFn0&e=> : > let mut log = true; + //This sanity check of the `mode` argument has already been done in `chmod_syscall()` before this helper funciton is called. + //Isn't it redundant to check the same thing here again? if mode & (S_IRWXA | (S_FILETYPEFLAGS as u32)) == mode { There is no mentioning anywhere that chmod is used to change file type. It is used for changing access mode bits. S_FILETYPEFLAGS is only used as a sanity check to make sure that we are working with the right types of files. This is also mentioned in the comments — Reply to this email directly, view it on GitHub <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Lind-2DProject_safeposix-2Drust_pull_275-23discussion-5Fr1650232706&d=DwMFaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=mwCephJ0TxjvnvPU0MlAEw&m=VO2tFj5V3hpn5LJQbM6WNXis7Pl-UofHAelqK455muSnPR1mNM5GAnXxswYR9Rrw&s=wMeuY5JxmBODJ794hjmF0UpknbtbGDWrW9MyILhoFn0&e=>, or unsubscribe <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AMW2TROKIVYC4WAYB2UPNMTZI5V5TAVCNFSM6AAAAABJXXZO5GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMZUGQ3DONBXGM&d=DwMFaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=mwCephJ0TxjvnvPU0MlAEw&m=VO2tFj5V3hpn5LJQbM6WNXis7Pl-UofHAelqK455muSnPR1mNM5GAnXxswYR9Rrw&s=GHDjtNepPictnEtdFzQTqy-TNvUCCV7cg_Ujfp01baU&e=> . You are receiving this because your review was requested.Message ID: ***@***.***>

[namanlalitnyu]

LGTM!

[rennergade]

This is a really good PR. All the comments are relevant and informative.

Approved!