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.

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

[closes #426] Apply AsBytes+FromBytes #466

Closed
wants to merge 2 commits into from

Conversation

coolofficials
Copy link
Collaborator

closes #426

off,
tx,
)?;
pub fn write_kernel<T: AsBytes + FromBytes>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

FromBytes는 없어도 되지 않나요? (UserMemory::copy_out도 마찬가지)

#[derive(Copy, Clone, AsBytes, FromBytes)]
// repr(packed) is required for AsBytes.
// https://docs.rs/zerocopy/0.3.0/zerocopy/trait.AsBytes.html
#[repr(packed)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

repr(C) 이외의 memory layout을 사용하면 안 됩니다. repr(packed)를 사용한 것으로 인해 ls에서 파일 크기가 제대로 출력되지 않습니다.

$ ls
.              1 1 0
..             1 1 0
README         2 2 0
cat            2 3 0
echo           2 4 0
forktest       2 5 0
grep           2 6 0
init           2 7 0
kill           2 8 0
ln             2 9 0
ls             2 10 0
mkdir          2 11 0
rm             2 12 0
sh             2 13 0
stressfs       2 14 0
usertests      2 15 0
grind          2 16 0
wc             2 17 0
zombie         2 18 0
console        3 19 0

repr(C)를 사용하되 nlink 다음에 padding을 위한 4 바이트 크기의 필드를 넣어서 해결하거나, copy_outwrite_kernel은 trait bound를 없애고 구현에서 unsafe를 사용해야 할 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

추가로, struct에서 padding에 해당하는 부분을 읽는 것이 Rust에서 UB인지 unspecified behavior인지 확인해 보는 것이 좋을 것 같습니다.

In other words, the only cases in which reading uninitialized memory is permitted are inside unions and in "padding" (the gaps between the fields/elements of a type).

(https://doc.rust-lang.org/reference/behavior-considered-undefined.html)

The value of padding bytes when storing values in structures or unions.

(C18 Spec: Unspecified behavior)

Rust reference의 설명을 봐도 그렇고, C에서 unspecified인 것을 봐도 그렇고 아마 UB는 아닌 것 같습니다만, 정확히 아시는 분이 계시면 좋을 것 같네요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

rust-lang/unsafe-code-guidelines#174
rust-lang/unsafe-code-guidelines#183

확실하지는 않지만 아마 괜찮을 것 같습니다

@coolofficials
Copy link
Collaborator Author

Conflict가 너무 많아서 수정사항 반영해서 주말 중에 다시 pr을 올리겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use FromBytes
2 participants