On 10/09/2014 02:36 AM, Magnus Reftel wrote: > This patch introduces the -seed command line option and the > QEMU_RAND_SEED environment variable for setting the random seed, which > is used for the AT_RANDOM ELF aux entry. > > Signed-off-by: Magnus Reftel <ref...@spotify.com> > ---
>
> +static void handle_arg_randseed(const char *arg)
> +{
> + unsigned long seed;
> + char* end;
Style: we prefer:
char *end;
> + seed = strtoul(arg, &end, 0);
> + if (end==arg || *end!='\0' || seed > UINT_MAX) {
Style: spaces around operators:
if (end == arg || *end || seed > UINT_MAX) {
Bug: strtoul() sometimes reports error via errno; the only safe way to
use it is to first prime errno = 0, then do strtoul, then check if errno
was changed.
Reimplementation: util/cutils.c already provides parse_uint() that takes
care of calling strtoul safely (hmm, that version only parses 64-bit
numbers; maybe we should expand it to also parse 32-bit numbers?)
Surprising behavior: your code behaves differently on 32-bit hosts than
it does on 64-bit hosts. Seriously. strotoul() has the annoying
specification of requiring twos-complement wraparound according to the
size of long, which means "-1" on a 32-bit platform parses as 0xffffffff
(accepted), while on a 64-bit platform parses it as 0xffffffffffffffff
(which you reject as > UINT_MAX); conversely "-18446744073709551615"
fails to parse due to overflow on a 32-bit platform, while successfully
being parsed as 1 on 64-bit.
> + fprintf(stderr, "Invalid seed number: %s\n", arg);
> + exit(1);
> + }
> + srand(seed);
> +}
> +
> static void handle_arg_gdb(const char *arg)
> {
> gdbstub_port = atoi(arg);
> @@ -3674,6 +3686,8 @@ static const struct qemu_argument arg_table[] = {
> "", "run in singlestep mode"},
> {"strace", "QEMU_STRACE", false, handle_arg_strace,
> "", "log system calls"},
> + {"seed", "QEMU_RAND_SEED", true, handle_arg_randseed,
> + "", "Seed for pseudo-random number generator"},
> {"version", "QEMU_VERSION", false, handle_arg_version,
> "", "display version information and exit"},
> {NULL, NULL, false, NULL, NULL, NULL}
> @@ -3856,6 +3870,8 @@ int main(int argc, char **argv, char **envp)
> cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
> #endif
>
> + srand(time(NULL));
> +
> optind = parse_args(argc, argv);
>
> /* Zero out regs */
> @@ -3926,6 +3942,10 @@ int main(int argc, char **argv, char **envp)
> do_strace = 1;
> }
>
> + if (getenv("QEMU_RAND_SEED")) {
> + handle_arg_randseed(getenv("QEMU_RAND_SEED"));
> + }
Now that you have exactly one caller of the static function, it might
make sense to just inline the body of that function here.
> +
> target_environ = envlist_to_environ(envlist, NULL);
> envlist_free(envlist);
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
