-
Notifications
You must be signed in to change notification settings - Fork 1
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
Establishing Coding Standards for lind-wasm #79
Comments
|
@yzhang71 Let's add all of this to the libc section of the docs. |
Are you implying there would be a single system call receiver where you then split out into different system calls? If so, you don't need this. The entry in the 3i table will go to the correct system call function. |
While working on adding new syscalls to lind-wasm and discussing with Qianxi, I noticed that due to the code being contributed by multiple people, there is some confusion regarding the specific code structure and formatting in the codebase. Different approaches have been used, and it’s not always consistent. Additionally, after discussing with Nick, we currently lack documentation outlining the coding standards for lind-wasm. Considering that we will have new contributors joining in the future, I think it would be helpful to have a discussion about what we would like the final coding standards to be and add them to documentation for lind-wasm.
Current solution for adding new syscalls
On the rawposix side:
dispatcher.rs:
On the glibc side:
glibc/include/unistd.h:
Modify visibility if necessary.sysdeps/unix/syscall.list:
Modify strong and weak names if necessary.sysdeps/unix/sysv/linux/
Key Considerations
1. Use of
__libc_
Prefix:Glibc uses the
__GI_
prefix to distinguish syscalls as part of its public API. I noticed that in previous reimplementations of syscalls in lind-wasm, we used the__libc_
prefix. Since I decided to exposereadlinkat
as public API asreadlink
and for consistency in the code base, I chose to adjust the naming accordingly (with the__libc_
prefix on both implementations). I’d like to confirm if this naming convention aligns with our long-term goals for the project.2. Inclusion of
sys-cancel.h
:While reviewing implementations of syscalls (e.g.,
open
), I noticed that many syscalls includesys-cancel.h
even those that are not directly blocking. From my understanding, this header is primarily used for handling thread cancellation. To maintain consistency and handle potential edge cases, I includedsys-cancel.h
in my implementation. I'd appreciate feedback on whether this is the best approach.3. Which
syscall.list
to Modify:From my understanding:
sysdeps/unix/syscalls.list
defines general Unix syscall that glibc supportssysdeps/unix/sysv/linux/syscalls.list
adds Linux-specific extensions or overrides the former.According to this, there are multiple approaches for exposing syscalls as public APIs. Below is the reasoning behind my thoughts.
In previous glibc implementation,
readlinkat
wasn’t directly exposed as a public API but was invoked viareadlink
(as seen with the__GI_
naming convention and original glibc source code). I modifiedsysdeps/unix/syscalls.list
to support exposing bothreadlink
andreadlinkat
as public APIs for lind-wasm.Considering whether
sysdeps/unix/sysv/linux/syscalls.list
might overwrite these changes, I think the weak aliasweak_alias(__libc_readlinkat, readlinkat)
inreadlinkat.c
should bindreadlinkat
to__libc_readlinkat
and prevent overwriting by original linux syscall.list. However, I’m not entirely certain if modifying onlysysdeps/unix/sysv/linux/syscalls.list
could achieve the same effect. I would appreciate any clarification on this matter.The text was updated successfully, but these errors were encountered: