Re: [PATCH v2 04/13] hw/mips/malta: Add re-usable rng_seed_hex_new() method

[Richard Henderson <richard.henderson@linaro.org>]

On 4/11/24 13:07, Richard Henderson wrote:
> On 4/11/24 03:15, Philippe Mathieu-Daudé wrote:
>> Extract common code from reinitialize_rng_seed() and
>> load_kernel() to rng_seed_hex_new().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/mips/malta.c | 20 ++++++++++++--------
>>   1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>> index af74008c82..9fc6a7d313 100644
>> --- a/hw/mips/malta.c
>> +++ b/hw/mips/malta.c
>> @@ -850,15 +850,24 @@ static void G_GNUC_PRINTF(3, 4) prom_set(uint32_t *prom_buf, int 
>> index,
>>       va_end(ap);
>>   }
>> -static void reinitialize_rng_seed(void *opaque)
>> +static char *rng_seed_hex_new(void)
>>   {
>> -    char *rng_seed_hex = opaque;
>>       uint8_t rng_seed[32];
>> +    char rng_seed_hex[sizeof(rng_seed) * 2 + 1];
>>       qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
>>       for (size_t i = 0; i < sizeof(rng_seed); ++i) {
>>           sprintf(rng_seed_hex + i * 2, "%02x", rng_seed[i]);
>>       }
>> +
>> +    return g_strdup(rng_seed_hex);
>> +}
>> +
>> +static void reinitialize_rng_seed(void *opaque)
>> +{
>> +    g_autofree char *rng_seed_hex = rng_seed_hex_new();
>> +
>> +    strcpy(opaque, rng_seed_hex);
>>   }
> 
> Though it isn't deprecated, strcpy isn't really any safer than sprintf.
> We don't need to be copying text around quite as much as this.
> 
> How about:
> 
> #define RNG_SEED_SIZE 32
> 
> static void rng_seed_hex_new(char buf[2 * RNG_SEED_SIZE + 1])
> {
>      static const char hex = "0123456789abcdef";
>      uint8_t rng_seed[RNG_SEED_SIZE];
> 
>      qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
>      for (int i = 0; i < RNG_SEED_SIZE; ++i) {
>          buf[i * 2 + 0] = hex[rng_seed[i] / 16];
>          buf[i * 2 + 1] = hex[rng_seed[i] % 16];

Hmm.  Maybe a

static inline char hexdump_nibble(unsigned val)
{
     return (val < 10 ? '0' : 'a') + val;
}

static inline void hexdump_byte(char *out, uint8_t byte)
{
     out[0] = hexdump_nibble(byte >> 4);
     out[1] = hexdump_nibble(byte & 15);
}

in "qemu/cutils.h", for use elsewhere including util/hexdump.c.


r~