[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH] lib/ukswrand: Remove private functions from public API
Hi Roxana,
On 13.07.2018 16:20, Roxana Nicolescu wrote:
Hi Simon,
When I first counter the problem, I made a simple main.c where I
generated my own private number generator.
And this was the result:
error: ‘uk_swrand’ undeclared (first use in this function)
The problem is that before link time, the generated object needs to know
the uk_swrand type.
I tried to change the definition of uk_swrand_init by passing a double
reference to struct uk_swrand. But, the constructor for uk_swrand_def
complained, and I presumed that this function should be private.
A solution for this would be to put the definition of uk_swrand
structure in swrand.h. Another option is to use the global random
generator uk_swrand_def and reinitialize it.
Ouhhh. Yes you are right. ;-)
I agree, the struct defintion should be exposed. You can send a patch
were you are doing this. Remember that the current definition depends on
the Config.uk option LIBUKSWRAND_MWC being selected. The idea was that a
user can choose which software random number generator should be
provided by the library. Multiply-with-carry is currently only available
but this may change in the future.
Thanks,
Simon
Thanks,
Roxana N.
On Fri, Jul 13, 2018 at 3:02 PM, Simon Kuenzer <simon.kuenzer@xxxxxxxxx
<mailto:simon.kuenzer@xxxxxxxxx>> wrote:
Hey Roxana,
On 11.07.2018 17:36, Roxana Nicolescu wrote:
By making the private uk_swrand_init_r and uk_swrand_randr_r
functions visible to users, they can call them when they are not
allowed to.
It was not intended to have uk_swrand_init_r() and
uk_swrand_randr_r() private. A use case is to initialize and operate
with your own (and maybe multiple random) number generators. Each
struct uk_swrand is used as storage to its current state. You may
want this to reduce contention (e.g., SMP environments).
For convenience, the library initializes one random number generator
already during boot.
Of course, there are always cases that you should not do with an
interface. A clear interface design (e.g., meaningful parameters and
fitting data types) and sometimes also description is required. We
usually prevent most of this mis-usage with UK_ASSERT() statements.
But even here in this particular case, whenever you re-initialize a
the number generator, nothing bad happens: It just starts throwing
out the same number sequence again.
In order to stop this to happen, these should be removed from
swrand.h
Also, because uk_swrand_randr function is the only public one,
the declaration static inline should be changed.
Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx
<mailto:nicolescu.roxana1996@xxxxxxxxx>>
---
lib/ukswrand/include/uk/swrand.h | 18 +-----------------
lib/ukswrand/mwc.c | 19 ++++++++++++++++---
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/lib/ukswrand/include/uk/swrand.h
b/lib/ukswrand/include/uk/swrand.h
index e2e43f9..7c4b35a 100644
--- a/lib/ukswrand/include/uk/swrand.h
+++ b/lib/ukswrand/include/uk/swrand.h
@@ -42,26 +42,10 @@
extern "C" {
#endif
-struct uk_swrand;
-extern struct uk_swrand uk_swrand_def;
-
-void uk_swrand_init_r(struct uk_swrand *r, __u32 seed);
-__u32 uk_swrand_randr_r(struct uk_swrand *r);
-
/* Uses the pre-initialized default generator */
/* TODO: Add assertion when we can test if we are in
interrupt context */
/* TODO: Revisit with multi-CPU support */
-static inline __u32 uk_swrand_randr(void)
-{
- unsigned long iflags;
- __u32 ret;
-
- iflags = ukplat_lcpu_save_irqf();
- ret = uk_swrand_randr_r(&uk_swrand_def);
- ukplat_lcpu_restore_irqf(iflags);
-
- return ret;
-}
+__u32 uk_swrand_randr(void);
#ifdef __cplusplus
}
diff --git a/lib/ukswrand/mwc.c b/lib/ukswrand/mwc.c
index 60a5bf1..493c455 100644
--- a/lib/ukswrand/mwc.c
+++ b/lib/ukswrand/mwc.c
@@ -37,6 +37,7 @@
#include <uk/print.h>
#include <uk/assert.h>
#include <uk/plat/ctors.h>
+#include <stdlib.h>
/*
https://stackoverflow.com/questions/9492581/c-random-number-generation-pure-c-code-no-libraries-or-functions
<https://stackoverflow.com/questions/9492581/c-random-number-generation-pure-c-code-no-libraries-or-functions>
*/
#define PHI 0x9e3779b9
@@ -55,12 +56,12 @@ struct uk_swrand uk_swrand_def;
*/
static void _uk_swrand_ctor(void)
__constructor_prio(UK_SWRAND_CTOR_PRIO);
-void uk_swrand_init_r(struct uk_swrand *r, __u32 seed)
+static void uk_swrand_init_r(struct uk_swrand *r, __u32 seed)
{
__u32 i;
UK_ASSERT(r);
-
+
r->Q[0] = seed;
r->Q[1] = seed + PHI;
r->Q[2] = seed + PHI + PHI;
@@ -71,7 +72,7 @@ void uk_swrand_init_r(struct uk_swrand *r,
__u32 seed)
r->i = 4095;
}
-__u32 uk_swrand_randr_r(struct uk_swrand *r)
+static __u32 uk_swrand_randr_r(struct uk_swrand *r)
{
__u64 t, a = 18782LL;
__u32 x, y = 0xfffffffe;
@@ -101,3 +102,15 @@ static void _uk_swrand_ctor(void)
uk_printd(DLVL_INFO, "Initialize random number
generator...\n");
uk_swrand_init_r(&uk_swrand_def,
CONFIG_LIBUKSWRAND_INITIALSEED);
}
+
+__u32 uk_swrand_randr(void)
Btw, without static keyword, you can still link to this symbol.
+{
+ unsigned long iflags;
+ __u32 ret;
+
+ iflags = ukplat_lcpu_save_irqf();
+ ret = uk_swrand_randr_r(&uk_swrand_def);
+ ukplat_lcpu_restore_irqf(iflags);
+
+ return ret;
+}
Thanks,
Simon
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
