[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.




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
        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
           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
        index e2e43f9..7c4b35a 100644
        --- a/lib/ukswrand/include/uk/swrand.h
        +++ b/lib/ukswrand/include/uk/swrand.h
        @@ -42,26 +42,10 @@
           extern "C" {
           -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>
           #define PHI 0x9e3779b9
        @@ -55,12 +56,12 @@ struct uk_swrand uk_swrand_def;
           static void _uk_swrand_ctor(void)
           -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;
                 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
        +__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;



Minios-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.