[PATCH] nandtest: use seed argument

[PATCH] nandtest: use seed argument

[Jan Weitzel]

if a seed is provided it is actually not used. First call is
"seed = rand()" killing the given seed.

Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>
---
 nandtest.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/nandtest.c b/nandtest.c
index db7f427..b3aacaf 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -173,6 +173,7 @@ int main(int argc, char **argv)
 
 		case 's':
 			seed = atol(optarg);
+			srand(seed);
 			break;
 
 		case 'p':
-- 
1.7.0.4

Re: [PATCH] nandtest: use seed argument

[Brian Norris]

On Thu, Nov 24, 2011 at 2:57 AM, Jan Weitzel <j.weitzel@phytec.de> wrote:
> if a seed is provided it is actually not used. First call is
> "seed = rand()" killing the given seed.
>
> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>

Tested-by: Brian Norris <computersforpeace@gmail.com>

You might also note that when a seed isn't provided, nandtest does not
produce random results because it just uses the default seed. At least
for my system, I see the same results every execution; I believe this
is the general C rand() behavior. I'll send a follow-up patch that
actually utilizes <time.h> to use time as a random seed.

Brian

[PATCH] nandtest: seed random generator properly

[Brian Norris]

This patch fixes two problems in nandtest:

(1) if a seed is provided it is actually not used. First call is
    "seed = rand()" killing the given seed.
    Credit: Jan Weitzel <j.weitzel@phytec.de>

(2) if a seed is not provided, we use the default rand() values, which
    produces the same sequence of values every run. It makes more sense
    to seed with the time to produce more random sequences.

Cc: Jan Weitzel <j.weitzel@phytec.de>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
Jan: This is an amendment to your patch. Feel free to add a
"Signed-off-by" if this works for you.

 nandtest.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/nandtest.c b/nandtest.c
index dc28d09..8cdc816 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -35,7 +35,7 @@ struct mtd_info_user meminfo;
 struct mtd_ecc_stats oldstats, newstats;
 int fd;
 int markbad=0;
-int seed;
+int seed = -1;
 
 int erase_and_write(loff_t ofs, unsigned char *data, unsigned char *rbuf)
 {
@@ -192,6 +192,10 @@ int main(int argc, char **argv)
 	if (argc - optind != 1)
 		usage();
 
+	if (seed < 0)
+		seed = time(NULL);
+	srand(seed);
+
 	fd = open(argv[optind], O_RDWR);
 	if (fd < 0) {
 		perror("open");
-- 
1.7.5.4

Antwort: [PATCH] nandtest: seed random generator properly

[Jan Weitzel]

Brian Norris <computersforpeace@gmail.com> schrieb am 28.11.2011 19:11:52:

> Von: Brian Norris <computersforpeace@gmail.com>
> An: <linux-mtd@lists.infradead.org>
> Kopie: Brian Norris <computersforpeace@gmail.com>, Artem Bityutskiy 
> <dedekind1@gmail.com>, Jan Weitzel <j.weitzel@phytec.de>
> Datum: 28.11.2011 19:13
> Betreff: [PATCH] nandtest: seed random generator properly
> 
> This patch fixes two problems in nandtest:
> 
> (1) if a seed is provided it is actually not used. First call is
>     "seed = rand()" killing the given seed.
>     Credit: Jan Weitzel <j.weitzel@phytec.de>
> 
> (2) if a seed is not provided, we use the default rand() values, which
>     produces the same sequence of values every run. It makes more sense
>     to seed with the time to produce more random sequences.
> 
> Cc: Jan Weitzel <j.weitzel@phytec.de>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
> Jan: This is an amendment to your patch. Feel free to add a
> "Signed-off-by" if this works for you.
> 
>  nandtest.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/nandtest.c b/nandtest.c
> index dc28d09..8cdc816 100644
> --- a/nandtest.c
> +++ b/nandtest.c
> @@ -35,7 +35,7 @@ struct mtd_info_user meminfo;
>  struct mtd_ecc_stats oldstats, newstats;
>  int fd;
>  int markbad=0;
> -int seed;
> +int seed = -1;
> 
>  int erase_and_write(loff_t ofs, unsigned char *data, unsigned char 
*rbuf)
>  {
> @@ -192,6 +192,10 @@ int main(int argc, char **argv)
>     if (argc - optind != 1)
>        usage();
> 
> +   if (seed < 0)
> +      seed = time(NULL);
> +   srand(seed);
> +

So you loose all negative seeds. What is about

int seed = time(NULL);
...
case 's'
        seed = atol(optarg);
...
}
srand(seed);

Jan



>     fd = open(argv[optind], O_RDWR);
>     if (fd < 0) {
>        perror("open");
> -- 
> 1.7.5.4
> 

Re: [PATCH] nandtest: seed random generator properly

[Brian Norris]

On Mon, Nov 28, 2011 at 11:30 PM, Jan Weitzel <J.Weitzel@phytec.de> wrote:
> Brian Norris <computersforpeace@gmail.com> schrieb am 28.11.2011 19:11:52:
>> +   if (seed < 0)
>> +      seed = time(NULL);
>> +   srand(seed);
>
> So you loose all negative seeds.

Well, srand() technically takes unsigned ints, so all seeds are
technically positive. But that just means we're parsing and storing
seeds wrong (not a big deal, really).

> What is about
>
> int seed = time(NULL);
> ...
> case 's'
>        seed = atol(optarg);
> ...
> }
> srand(seed);

Not sure if I understand the question perfectly (clarify if I'm
wrong). My patch was intended to:
(1) use the user-supplied seed (--seed=X) if supplied
(2) use a random seed (based on time()) if the seed is left
uninitialized (i.e., -1)

Anyway, I see a problem with my method. How about the following patch,
which saves the time first, then overwrites it with the supplied seed
if provided.

>From 4df0643c4819f5de5db54b855406852da0dc6bf1 Mon Sep 17 00:00:00 2001
From: Brian Norris <computersforpeace@gmail.com>
Date: Mon, 28 Nov 2011 10:03:30 -0800
Subject: [PATCH] nandtest: seed random generator properly

This patch fixes two problems in nandtest:

* if a seed is provided it is actually not used. First call is
  "seed = rand()" killing the given seed.
  Credit: Jan Weitzel <j.weitzel@phytec.de>

* if a seed is not provided, we use the default rand() values, which
  produces the same sequence of values every run.r

To solve these problems, we seed the random number generator with either
the time() or the supplied seed.

Cc: Jan Weitzel <j.weitzel@phytec.de>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 nandtest.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/nandtest.c b/nandtest.c
index dc28d09..7811be2 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -140,6 +140,8 @@ int main(int argc, char **argv)
        uint32_t offset = 0;
        uint32_t length = -1;

+       seed = time(NULL);
+
        for (;;) {
                static const char *short_options="hkl:mo:p:s:";
                static const struct option long_options[] = {
@@ -243,6 +245,8 @@ int main(int argc, char **argv)
        printf("Bad blocks     : %d\n", oldstats.badblocks);
        printf("BBT blocks     : %d\n", oldstats.bbtblocks);

+       srand(seed);
+
        for (pass = 0; pass < nr_passes; pass++) {
                loff_t test_ofs;

-- 
1.7.5.4

Re: [PATCH] nandtest: use seed argument

[Artem Bityutskiy]

On Thu, 2011-11-24 at 11:57 +0100, Jan Weitzel wrote:
> if a seed is provided it is actually not used. First call is
> "seed = rand()" killing the given seed.
> 
> Signed-off-by: Jan Weitzel <j.weitzel@phytec.de>

Pushed to mtd-utils.git, thanks!

Artem.

Re: [PATCH] nandtest: seed random generator properly

[Jan Weitzel]

Am Mittwoch, den 30.11.2011, 10:24 -0800 schrieb Brian Norris:
> On Mon, Nov 28, 2011 at 11:30 PM, Jan Weitzel <J.Weitzel@phytec.de> wrote:
> > Brian Norris <computersforpeace@gmail.com> schrieb am 28.11.2011 19:11:52:
> >> +   if (seed < 0)
> >> +      seed = time(NULL);
> >> +   srand(seed);
> >
> > So you loose all negative seeds.
> 
> Well, srand() technically takes unsigned ints, so all seeds are
> technically positive. But that just means we're parsing and storing
> seeds wrong (not a big deal, really).
> 
> > What is about
> >
> > int seed = time(NULL);
> > ...
> > case 's'
> >        seed = atol(optarg);
> > ...
> > }
> > srand(seed);
> 
> Not sure if I understand the question perfectly (clarify if I'm
> wrong). My patch was intended to:
> (1) use the user-supplied seed (--seed=X) if supplied
> (2) use a random seed (based on time()) if the seed is left
> uninitialized (i.e., -1)
> 
> Anyway, I see a problem with my method. How about the following patch,
> which saves the time first, then overwrites it with the supplied seed
> if provided.
Acked-by: Jan Weitzel <j.weitzel@phytec.de>

Maybe you need to rebase it, because my former patch just got pushed.

> From 4df0643c4819f5de5db54b855406852da0dc6bf1 Mon Sep 17 00:00:00 2001
> From: Brian Norris <computersforpeace@gmail.com>
> Date: Mon, 28 Nov 2011 10:03:30 -0800
> Subject: [PATCH] nandtest: seed random generator properly
> 
> This patch fixes two problems in nandtest:
> 
> * if a seed is provided it is actually not used. First call is
>   "seed = rand()" killing the given seed.
>   Credit: Jan Weitzel <j.weitzel@phytec.de>
> 
> * if a seed is not provided, we use the default rand() values, which
>   produces the same sequence of values every run.r
> 
> To solve these problems, we seed the random number generator with either
> the time() or the supplied seed.
> 
> Cc: Jan Weitzel <j.weitzel@phytec.de>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  nandtest.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/nandtest.c b/nandtest.c
> index dc28d09..7811be2 100644
> --- a/nandtest.c
> +++ b/nandtest.c
> @@ -140,6 +140,8 @@ int main(int argc, char **argv)
>         uint32_t offset = 0;
>         uint32_t length = -1;
> 
> +       seed = time(NULL);
> +
>         for (;;) {
>                 static const char *short_options="hkl:mo:p:s:";
>                 static const struct option long_options[] = {
> @@ -243,6 +245,8 @@ int main(int argc, char **argv)
>         printf("Bad blocks     : %d\n", oldstats.badblocks);
>         printf("BBT blocks     : %d\n", oldstats.bbtblocks);
> 
> +       srand(seed);
> +
>         for (pass = 0; pass < nr_passes; pass++) {
>                 loff_t test_ofs;
> 

Re: [PATCH] nandtest: seed random generator properly

[Artem Bityutskiy]

On Wed, 2011-11-30 at 10:24 -0800, Brian Norris wrote:
> On Mon, Nov 28, 2011 at 11:30 PM, Jan Weitzel <J.Weitzel@phytec.de> wrote:
> > Brian Norris <computersforpeace@gmail.com> schrieb am 28.11.2011 19:11:52:
> >> +   if (seed < 0)
> >> +      seed = time(NULL);
> >> +   srand(seed);
> >
> > So you loose all negative seeds.
> 
> Well, srand() technically takes unsigned ints, so all seeds are
> technically positive. But that just means we're parsing and storing
> seeds wrong (not a big deal, really).

Brian, would you please send an incremental patch on top of Jan's which
I've pushed?

Artem.

[PATCH v2] nandtest: seed random generator with time

[Brian Norris]

If a seed is not provided via --seed, we use the default rand() values,
which produces the same sequence of values every run. Since this is
undesirable, we should choose a random seed via the current time().

Note that this patch moves the srand() until after all the initial
options processing.

Cc: Jan Weitzel <j.weitzel@phytec.de>
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
rebased on top of Jan's patch

 nandtest.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/nandtest.c b/nandtest.c
index b3aacaf..0187b87 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -140,6 +140,8 @@ int main(int argc, char **argv)
 	uint32_t offset = 0;
 	uint32_t length = -1;
 
+	seed = time(NULL);
+
 	for (;;) {
 		static const char *short_options="hkl:mo:p:s:";
 		static const struct option long_options[] = {
@@ -173,7 +175,6 @@ int main(int argc, char **argv)
 
 		case 's':
 			seed = atol(optarg);
-			srand(seed);
 			break;
 
 		case 'p':
@@ -244,6 +245,8 @@ int main(int argc, char **argv)
 	printf("Bad blocks     : %d\n", oldstats.badblocks);
 	printf("BBT blocks     : %d\n", oldstats.bbtblocks);
 
+	srand(seed);
+
 	for (pass = 0; pass < nr_passes; pass++) {
 		loff_t test_ofs;
 
-- 
1.7.5.4

Re: [PATCH v2] nandtest: seed random generator with time

[Artem Bityutskiy]

On Fri, 2011-12-02 at 09:46 -0800, Brian Norris wrote:
> If a seed is not provided via --seed, we use the default rand() values,
> which produces the same sequence of values every run. Since this is
> undesirable, we should choose a random seed via the current time().
> 
> Note that this patch moves the srand() until after all the initial
> options processing.

Pushed, thanks!

Artem.