From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XcFd5-0007jn-IP for qemu-devel@nongnu.org; Thu, 09 Oct 2014 11:27:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XcFd0-0003Da-Ma for qemu-devel@nongnu.org; Thu, 09 Oct 2014 11:27:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:16918) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XcFd0-0003DP-FJ for qemu-devel@nongnu.org; Thu, 09 Oct 2014 11:27:42 -0400 Message-ID: <5436A96B.6050708@redhat.com> Date: Thu, 09 Oct 2014 09:27:39 -0600 From: Eric Blake MIME-Version: 1.0 References: <1412843785-960-1-git-send-email-reftel@spotify.com> <1412843785-960-2-git-send-email-reftel@spotify.com> In-Reply-To: <1412843785-960-2-git-send-email-reftel@spotify.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="X9AFwGbJ69KWXGuSs2wtUCPaUEvPG9KL2" Subject: Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Magnus Reftel , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --X9AFwGbJ69KWXGuSs2wtUCPaUEvPG9KL2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Magnus Reftel > --- > =20 > +static void handle_arg_randseed(const char *arg) > +{ > + unsigned long seed; > + char* end; Style: we prefer: char *end; > + seed =3D strtoul(arg, &end, 0); > + if (end=3D=3Darg || *end!=3D'\0' || seed > UINT_MAX) { Style: spaces around operators: if (end =3D=3D arg || *end || seed > UINT_MAX) { Bug: strtoul() sometimes reports error via errno; the only safe way to use it is to first prime errno =3D 0, then do strtoul, then check if errn= o 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 =3D atoi(arg); > @@ -3674,6 +3686,8 @@ static const struct qemu_argument arg_table[] =3D= { > "", "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 (TB= D) */ > #endif > =20 > + srand(time(NULL)); > + > optind =3D parse_args(argc, argv); > =20 > /* Zero out regs */ > @@ -3926,6 +3942,10 @@ int main(int argc, char **argv, char **envp) > do_strace =3D 1; > } > =20 > + 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 =3D envlist_to_environ(envlist, NULL); > envlist_free(envlist); > =20 >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --X9AFwGbJ69KWXGuSs2wtUCPaUEvPG9KL2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUNqlrAAoJEKeha0olJ0Nq+iYH/A8Xt9KyuxOTfgBgbx/R1bPe /eGL7X8802lUYQ0OwHWZbHGRqi0gtIzTK+ESvoNu+sgq5S8Wv4XphepxG8JuFvnX 3zlIS6vg9BK6Sm1Ij5lVWncWlUy1uTCVadZFKephPs4pUcB0Cr+WVpVvLdv+vuqI CtRhsJr/4DEXQHN4NDAJbz+aDBXMLeLghY6ncMr/avFgY9fv45RhEwJbmEsdJUzD yI4K84xYK4fj1MfPR5YnU3LtbgLtF2s+cB4avtZegh4cngjNTbwm0llm0dyuJSyL ZpUYT223ohx251Sdw1Oyjvx/wDDQP7IHDxqw3QHRjIfqsY/+t/qGz9N+7g5V0mE= =S3fB -----END PGP SIGNATURE----- --X9AFwGbJ69KWXGuSs2wtUCPaUEvPG9KL2--