libc/minimal: Add rand and srand

[yashi]

rand() and srand() are pseudo-random number generator functions
defined in ISO C. This implementation uses the Linear congruential
generator algorithm with the following parameters, which are the same
as GNU Libc.

Modulus 0x7fffffff
Multiplier 1103515245
Increment 12345

ISO C / POSIX.1-2001 uses bits high 15 bits for Modulus, thus RAND_MAX
32767. ref rand(3).

Signed-off-by: Yasushi SHOJI [email protected]

[yashi]

Or should I call sys_rand32_get(), instead?
https://docs.zephyrproject.org/latest/reference/random/index.html

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

[yashi]

@enjiamai, @KangJianX would mind to give me some feedback?

[enjiamai]

Hi @yashi , OK, please give me some time to take look at this, I'll feedback to you later. thanks!

[enjiamai]

Hi @yashi , I have no problem with this implementation, it looks good, and I think we need this. I would so appreciate it if some basic test cases for this can also be added to tests/lib/c_lib/src/. How do you think? thank you!

[enjiamai]

Could you please help to add the test cases for these, thank you!

[enjiamai]

Looks great~ no problem from my side, thanks!

[pfalcon]

I'm always +1 for sustainably growing the minimal library. But IMHO there're few cleanups to be done and questions to be answered before this is ready for inclusion.

Another note - we usually try to put tests into a separate commit vs the core changes.

[pfalcon]

I also find the current commit message to be confusing:

            initstate(0, state, sizeof(state));
            printf("%d\n", rand());

            return 0;
    }

See initstate(3p) for more detail.

You don't implement initstate(), so why people should see its man? I however followed the advice and what I see is:

       long int random(void);

       void srandom(unsigned int seed);

       char *initstate(unsigned int seed, char *state, size_t n);

       char *setstate(char *state);

In other words, rand/srand vs random/initstate seems to be two parallel, independent pseudo-random number generation function sets. You here don't implement the latter, so references to initstate() should rather be removed. Likewise, please double-check that rand/srand implementation matches that of glibc. If it is, please update the sample in the commit message to use rand/srand. And if it's not, I'd suggest to just use the sample implementation from POSIX: https://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html and remove references to glibc.

(And if there's some confusion on my side, please let me know what I'm missing.)

[yashi]

In order to test the Linear congruential generator algorithm, which is refered as TYPE_0 in GLIBC, you have to call initstate() with a state array of size 8 bytes or smaller than 32 byte. Please note that the man page you referred is 3 not 3p, or 3posix, from Linux man-pages project.

I've updated my commit log as below for more clarity.

See initstate(3p) for more detail about how to use LCG in GLIBC.

[pfalcon]

@yashi

In order to test the Linear congruential generator algorithm, which is refered as TYPE_0 in GLIBC, you have to call initstate() with a state array of size 8 bytes or smaller than 32 byte.

That's an interesting implementation detail of GLIBC, but I don't see how that relates to the functions srand()/rand() which are being implemented here. Nothing in https://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html mentions initstate(). Nor you implement initstate(). So, why do you mention it at all?

Let's start from the beginning. Do you want Zephyr's implementation of srand()/rand() to have the same behavior/produce the same values as Glibc's srand()/rand()? If so, I'm generally +1 for that, as Glibc is a well known library and I'm for being consistent with well-know de-facto standard. But then please sure that you exactly follow behavior of srand()/rand() and don't involve unrelated implementation details, like the fact that in Glibc, srand() is implemented in terms of initstats().

And well, if you don't have that specific requirement to follow Glibc behavior, just use the simple algorithm from "Generating the Same Sequence on Different Machines" subheading of https://pubs.opengroup.org/onlinepubs/9699919799/functions/rand.html .

Please note that the man page you referred is 3 not 3p, or 3posix, from Linux man-pages project.

That page still described the functions:

       char *initstate(unsigned seed, char *state, size_t size);
       long random(void);
       char *setstate(char *state);
       void srandom(unsigned seed);

And not srand()/rand().

[yashi]

@pfalcon, thanks for your feedback. The reason why I've chosen GLIBC LCG is that I assumed most of the developers in the Zephyr community prefer to have the same algorithm as their development environment, which is, I also assume, Linux with GLIBC. I don't have any preferences at all. And the reason I put the reference in the commit log is that when someone wants to verify my implementation, it'd be nice to give a point or two for them. I thought it'd be helpful for future references. Nothing else.

I followed GLIBC LCG's behavior in the Zephyr implementation. No detail, nor TYPE_0 thing is in Zephyr implementation. The reference is just for a hint to who wants verification in behavior. If the default algorithm in GLIBC is LCG, I didn't put the reference in the commit log, but it's not. If you just run rand() and srand() on your Linux, you are using a different algorithm and you get different values.

Again, I don't have any preferences in the algorithms. I just want rand() and srand() for compatibility. I can either remove the reference from the commit log or implement the C standard. Please let me know which way you want.

[pfalcon]

Is this include really needed?

[yashi]

nop. I'll leave stdlib.h there for checking the rand() and srand() declarations against the definitions.

[yashi]

@stephanosio, I saw the MAINTAINERS.yml change and you are now the maintainer of libcs. Would you mind reviewing this PR? I just rebased to the main. Thanks.

[cfriedt]

@yashi - can you please address any remaining comments?

[yashi]

Yes. Will do it in this weekend.

[stephanosio]

Some additional comments

[cfriedt]

fixes #6493

[stephanosio]

LGTM