Luca,

(not an HAProxy maintainer)

please have a look at the CONTRIBUTING file regarding the commit message
format: https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING

You are missing the severity indicator and the affected subsystem. It
probably should be `MINOR: sample:`.

> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index ba5a8b360..e22557134 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -14391,6 +14391,11 @@ thread : integer
>    the function, between 0 and (global.nbthread-1). This is useful for logging
>    and debugging purposes.
>  
> +uuid([<version>]) : string
> +  Returns a random UUID, following the RFC4122 standard. If the version is 
> not
               ^^^^^^
I'd remove the random here. UUID v3 while not currently supported can
hardly be called "random". UUID in itself already implies "unique".

> +  specified, a UUID version 4 (fully random) is returned.
> +  Currently, only version 4 is supported.
> +
>  var(<var-name>) : undefined
>    Returns a variable with the stored type. If the variable is not set, the
>    sample fetch fails. The name of the variable starts with an indication
> diff --git a/src/sample.c b/src/sample.c
> index 6327b6b08..6caa440b7 100644
> --- a/src/sample.c
> +++ b/src/sample.c
> @@ -3196,6 +3196,47 @@ static int smp_fetch_const_meth(const struct arg 
> *args, struct sample *smp, cons
>       return 1;
>  }
>  
> +// Generate a RFC4122 UUID (default is v4 = fully random)
> +static int
> +smp_fetch_uuid(const struct arg *args, struct sample *smp, const char *kw, 
> void *private)
> +{
> +    if (!args[0].data.sint || args[0].data.sint == 4) {
> +        uint32_t rnd[4] = { 0, 0, 0, 0 };
> +        uint64_t last = 0;
> +        int byte = 0;
> +        uint8_t bits = 0;
> +        unsigned int rand_max_bits = my_flsl(RAND_MAX);
> +
> +        while (byte < 4) {
> +            while (bits < 32) {
> +                last |= (uint64_t)random() << bits;
> +                bits += rand_max_bits;
> +            }
> +            rnd[byte++] = last;
> +            last >>= 32u;
> +            bits  -= 32;
> +        }
> +
> +        chunk_printf(&trash, "%8.8x-%4.4x-4%3.3x-%4.4x-%12.12llx",
                                             ^
Instead of hardcoding the 4 here (it's easy to miss within all the
digits for the format specifiers) I suggest to instead "force" the bits
to match the version within the random values using bit operations.

> +                     rnd[0], // first 32 bits
> +                     rnd[1] % 65536, // getting fist 16 bits
                                 ^^^^^             ^^^^
Hexadecimal notation and a bit mask would capture the intent better:
rand[1] & 0xFFFF. The comment is redundant then.

Typo in "first".

> +                     (rnd[1] >> 16u) % 4096,  // getting bits 17 - 28 (12 
> bits), leaving out bits 29 - 32

With the suggestion regarding the version number this turns into:

((rand[1] >> 16u) & 0xFFF) | 0x4000 // the highest 4 bits indicate the
UUID version

> +                     (rnd[2] % 16384) + 32768,  // getting first 14 bits, 
> setting the first two bits to 1 and 0...

I'd do (rnd[2] & 0x3FFF) | 0x8000. // the highest 2 bits indicate the
UUID variant (10)

> +                     (*(((uint64_t*) rnd) + 1) >> 14u) % 281474976710656 // 
> getting 48 bits (last 18 of rnd[2] + first 30 of rnd[3], 281474976710656 = 
> 2^48)

Instead of casting this to a 64 bit value I guess it's easier to
understand if you add the remaining 48 bits in two steps, the remaining
18 of the 3rd byte and the 30 bits of the 4th byte.

> +        );
> +
> +        smp->data.type = SMP_T_STR;
> +        smp->flags = SMP_F_VOL_TEST | SMP_F_MAY_CHANGE;;
                                                         ^^
Duplicate semicolon.

> +        smp->data.u.str = trash;
> +
> +        return 1;
> +    }
> +
> +    // more implementations of other uuid formats possible here
> +    return 0;
> +
> +}
> +
>  /* Note: must not be declared <const> as its list will be overwritten.
>   * Note: fetches that may return multiple types must be declared as the 
> lowest
>   * common denominator, the type that can be casted into all other ones. For
> @@ -3214,6 +3255,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, {
>       { "rand",         smp_fetch_rand,  ARG1(0,SINT), NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
>       { "stopping",     smp_fetch_stopping, 0,         NULL, SMP_T_BOOL, 
> SMP_USE_INTRN },
>       { "stopping",     smp_fetch_stopping, 0,         NULL, SMP_T_BOOL, 
> SMP_USE_INTRN },
> +     { "uuid",         smp_fetch_uuid,  ARG1(0, SINT),      NULL, SMP_T_STR, 
> SMP_USE_INTRN },
>  
>       { "cpu_calls",    smp_fetch_cpu_calls,  0,       NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
>       { "cpu_ns_avg",   smp_fetch_cpu_ns_avg, 0,       NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
> -- 
> 2.23.0


Reply via email to