SDL_malloc uses simpler api if available to initialize #7374
Conversation
|
glibc added the arc4random functions in July 2022. Adding this makes it possible to build a SDL library on a modern glibc, but not runnable on older systems. |
Oh fair enough, well I did that I had BSD/macOs in mind anyway. |
CMakeLists.txt
Outdated
| @@ -1045,6 +1045,10 @@ if(SDL_LIBC) | |||
| check_symbol_exists(elf_aux_info "sys/auxv.h" HAVE_ELF_AUX_INFO) | |||
| check_symbol_exists(poll "poll.h" HAVE_POLL) | |||
|
|
|||
| if (NOT CMAKE_SYSTEM_NAME MATCHES "Linux") | |||
| check_symbol_exists(arc4random_buf "stdlib.h" HAVE_ARC4RANDOM) | |||
There was a problem hiding this comment.
I'm okay with always detecting the symbol, but would guard it in c using similar logic seen in php/php-src#10390
There was a problem hiding this comment.
fair point but I thought of the case if it was, one day, outside f just SDL_malloc, then just dismiss it only one place..
There was a problem hiding this comment.
Perhaps do this in SDL_build_config.h.cmake instead?
devnexen
force-pushed
the
arc4random_use_dev_random
branch
from
0bb28ff to
b9e710d
Compare
|
/dev/random is a better source of entropy than the arc4 random code. Is there a specific reason this change is important? |
|
offering a file descriptor-less solution. indeed usually getrandom, reading /dev/Urandom is better when there is need for strong entropy but I thought that might be far enough for this use case. |
|
Yeah, I'm not sure about the security requirements here, but it definitely seems better than something based on time() |
| @@ -208,6 +208,12 @@ | |||
| #cmakedefine HAVE_POLL 1 | |||
| #cmakedefine HAVE__EXIT 1 | |||
|
|
|||
| #if !defined(__linux__) | |||
| /* Disabling for Linux at least for now until the feature matures | |||
| a bit on this platform. */ | |||
There was a problem hiding this comment.
This doesn't look good. I know @madebr expressed concerns about compatibility
of library built on newer systems would be inoperable on older ones, but that
is a well known story anyway. So, if this patch is going to go in, and if we
really want to address that, I suggest making this a compile time option in
cmake rather than explicitly disabling it for linux.
Besides, this part of the code isn't even enabled by default, so I don't know
why is there sof much effont on it...
There was a problem hiding this comment.
Yes, in general we do build-time checks for system functionality and we don't guarantee that something built on one version of Linux will work on an earlier version.
To @sezero's point, is this code that will be run in SDL?
There was a problem hiding this comment.
To @sezero's point, is this code that will be run in SDL?
Unless SDL_malloc.c is explicitly patched, no: The patch changes code guarded
by FOOTERS which isn't defined by default (neither is USE_DEV_RANDOM)
|
Since FOOTERS isn't defined by default, this patch doesn't really improve things in SDL. I would recommend submitting it upstream to the dlmalloc author. |
the magic id.