-
Notifications
You must be signed in to change notification settings - Fork 375
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
Made srand(...) and rand() platform independent #711
Made srand(...) and rand() platform independent #711
Conversation
19b5647
to
e5dbff6
Compare
I noticed that the messages of all previous commits are also checked (which kept failing), so I deleted the previous commits, leaving only the last commit in the history. |
Can you share on which (embedded-)platform you are running Wakaama? It looks like you still include I'm wondering what's the best way how to deal with exotic platforms in general. I don't think it's a good practice to re-implement and wrap standard C functions. In my opinion we should assume that we can rely at least on the ISO C standard library (and POSIX?). However the project does not state any minimal requirements yet. Might be worth a discussion #494. If (s)rand() is not available on your platform you might implement your own rand() function without the wrapper? I guess the same goes for malloc(), free() etc.. |
Sorry it's my mistake, I shouldn't have said it's not available. The problem with this pseudo random generator is that it relies on the seed, so given a seed, the In this specific case Wakaama uses the timestamp returned by At the moment, at every boot, I connect to an NTP server (time.google.com) to synchronize the clock, but the problem remains, because it can happen that I am under a network where the rules prevent me from accessing certain IP addresses (e.g. China?), or the NTP service no longer works... For this reason, manufacturers usually provide proprietary functions/APIs that generate a real random number (as in my case). I am working with cellular modem (BG95-M5) which runs with ThreadX, a non-POSIX platform. Another solution could be the following: lwm2m_context_t * lwm2m_init(void * userData)
{
lwm2m_context_t * contextP;
LOG("Entering");
contextP = (lwm2m_context_t *)lwm2m_malloc(sizeof(lwm2m_context_t));
if (NULL != contextP)
{
memset(contextP, 0, sizeof(lwm2m_context_t));
contextP->userData = userData;
srand((int)lwm2m_seed());
contextP->nextMID = rand();
}
return contextP;
} |
Even if I like the idea to build platform independent software. I would try to reduce the customization possibilities in the code. I would prefer to leverage the possibilities of the toolchain (compiler, linker...) or the OS (loader) for such customization. These tools have all we need and it reduces some unneccessary complexety of our code. If you want to use your custom
The linker will pick your implementation instead the one from the standard library. |
I understand your point of view. But I think the problem is still there. Perhaps it would be better to replace with: srand((int)lwm2m_seed()); And let the user define the |
I see your point. So probably we need a possibility to customise |
@parmi93 all valid points, thanks for clarifying. I like your proposed solution to have a custom seed function.
|
I need a hint, what can I write in the int lwm2m_seed(void)
{
//what should I return?
} I was thinking of something that works on POSIX (Linux) platforms, and fails to compile on non-Linux platforms, so anyone implementing this library on a non-Linux platform would have to look here and fix this error by providing a reliable seed. |
I would suggest to keep |
1876608
to
c571f40
Compare
Commits squashed and commit message subject line changed. |
928d4fc
to
fb652f6
Compare
The changes look good. Now the CI is still complaining about formatting. You can use
|
The lwm2m_seed() function is used to obtain a seed for random number generation. The implementation of the lwm2m_seed() function is up to the user.
e801a79
to
f2cfde0
Compare
Code formatted with |
The
srand(...)
andrand()
functions are not available on all embedded platforms, so they were replaced withlwm2m_srand(...)
andlwm2m_rand()
to make these calls platform independent.