Hey, sorry, didn't know about these guidelines. I packed all changes into one commit in the patch file in the attachments. I included your changes, you are right, the code looks much better now.
I'm open for any feedback.
Best regards,
Luca
On 10.09.19, 14:25, "Tim Düsterhus" <t...@bastelstu.be> wrote:
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
0001-MINOR-sample-Add-UUID-fetch.patch
Description: 0001-MINOR-sample-Add-UUID-fetch.patch
