Doing some #ifdef cleanup for drand48() #22962
Conversation
|
I added a simple fix from the discussion in #22968. If it should be in a separate PR let me know. |
|
Anything else I can do on this? Is this something we want to move forward on? |
| @@ -4693,26 +4693,13 @@ Perl_seed(pTHX) | |||
| * value from (tv_sec * SEED_C1 + tv_usec). The multipliers should | |||
| * probably be bigger too. | |||
| */ | |||
| #if RANDBITS > 16 | |||
There was a problem hiding this comment.
I'd like to see this check replaced by something like
STATIC_ASSERT_DECL(RANDBITS == 48); /* hardcoded in Configure */so we don't silently get garbage if someone changes it.
| # ifdef __amigaos4__ | ||
| # define PERL_RANDOM_DEVICE "RANDOM:SIZE=4" | ||
| # else | ||
| # define PERL_RANDOM_DEVICE "/dev/urandom" | ||
| # endif |
There was a problem hiding this comment.
I don't like this change. If we don't support AmigaOS 4 anymore (is there an official platform list somewhere?), we should rip out all #ifdef __amigaos4__ code in this file (as well as the amigaos4/ subdirectory, and the hints/amigaos.sh file, and README.amiga, and pod/perlamiga.pod, and ext/Amiga-ARexx, and ext/Amiga-Exec).
There was a problem hiding this comment.
This pretty easy to revert (and harmless). When I checked the list of supported platforms I didn't see AmigaOS in the list. Worst case scenario if we remove this, then AmigaOS would just get it's seed like all the other platforms without /dev/urandom/
I think we just need guidance on what platforms are officially supported or not.
The code for random initialization in
util.cis riddled with a lot of exceptions in the form of#ifdefs. The code is actually quite complicated to read because of all the ifdefs. This pull request removesifdefsfor Plan9 and AmigaOS 4.x. This should simplify the code, and make it more readable.This PR also removes support for
RAND_BITS < 16which, from what I can tell is only appropriate on Plan9? It is probably safe to remove as well? That section could be removed from the PR if we still need it. As far as I know we don't actively support any 16 bit platforms anymore.