Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Overall this patch is almost there, it just needs a couple more mechanical things since it adds a new header. Specifically, you should add the header definition to spec/linux.td. You can look at how HeaderSpecs are defined in spec/stdc.td, I would recommend looking at the one for string.h as an example, or feel free to reach out if you need help figuring it out.
libc/include/CMakeLists.txt | ||
---|---|---|
237 | you need to add this target to include/llvm-libc-macros/CMakeLists.txt (it should be the same as the others in there) as well as the cmake file in the linux subfolder. | |
libc/src/sys/CMakeLists.txt | ||
8 | nit: please fix | |
libc/src/sys/random/getrandom.h | ||
2–3 | nit: this should all be on one line | |
libc/src/sys/random/linux/CMakeLists.txt | ||
13 | nit: please fix | |
libc/src/sys/random/linux/getrandom.cpp | ||
22 | if there's only one possible syscall, we usually don't bother with checking if it's defined, since the compiler will do that for us. | |
libc/test/src/sys/random/linux/getrandom_test.cpp | ||
48 | nit: please fix |
- address code reviews:
- add getrandom to linux specification
- add sys/random.h to header target
- random format bits
Thanks for the patch and working through all the details. Overall LGTM but needs one code policy related change.
libc/include/llvm-libc-macros/linux/sys-random-macros.h | ||
---|---|---|
16 | An empty line between 15 and 16? | |
libc/test/src/sys/random/linux/getrandom_test.cpp | ||
6 | We don't include C++ headers from unit tests. Also, for functions like fabs, we use internal functions. So:
|
- address code reviews
- add blank lines in headers
- use internal math functions in test
- let getrandom_test depend on lib.src.math.fabs
- additional fix
- fix wrong names in macro headers (rename STAT to RANDOM)
@michaelrj would you please consider merging this or give more instructions on updating this?
I see that some syscall related changes may have already made this patch incompatible. I am worried that it will become even harder to keep track given the active development of libc.
- rebase to main
- address code reviews:
- use syscall_impl instead
- add SysRandom to linux spec file
you need to add this target to include/llvm-libc-macros/CMakeLists.txt (it should be the same as the others in there) as well as the cmake file in the linux subfolder.