freaky error

[Bruno Calado]

When I compile this code without the printf line there is no random numbers generation!

What hell is that?

tks

include <stdio.h>

include <stdlib.h>

include <time.h>

//FUNÇÕES
unsigned short int myrand();

int main()
{
int w, var[10];

for(w=0; w<10; w++) {
    var[w]=0;
}
int tmp2=0;
for(w=0; w<10000; w++) {
    tmp2=myrand();

>>>>>>>>>>>>> printf("> %d\n",tmp2);
var[tmp2]++;
}

for(w=0; w<10; w++) {
    printf("%d: %d\n",w,var[w]);
}

getchar();

return 0;
}

unsigned short int myrand() {
static int bit_inicio=0;
if(!bit_inicio) {
srand(1); //CARREGA MATRIX GERADORA DE RANDOM NUMBERS
bit_inicio=1;
}
unsigned short int tmp;
while (tmp>9) {
tmp = rand();
}

return tmp;

}

[cpns]

Here:

unsigned short int tmp;
while (tmp>9) {
tmp = rand();
}

tmp is used before it is initialised.

Either:

unsigned short int tmp ;
do
{
tmp = rand();
} while (tmp>9) ;

But this is a really bad way of getting a value 0 to 8!

Why not:

unsigned short int tmp ;
tmp = rand() % 9 ;

? It is not a statistically robust method, but generally when the range is much less than RAND_MAX it is good enough for moat applications for which rand() is even suitable in the first place.

Clifford

[cpns]

Sorry my error (or rather confusion by your odd code!), the code returns 0 to 9 not 8.

unsigned short int tmp ;
tmp = rand() % 10 ;

or just:

return rand() % 10 ;

the tmp variable is entirely redundant.

To clarify, the printf() was a red herring; in fact, when I built it, it produced the correct output. The value of an unitialised variable are non-deterministic, it will be whatever happend to be at that memory location. When you change the code, you are likely to change the the address of the variable, and therefore its unitialised content. The difference between working and not working depended entirely on tmp initially containing less than 10 or 10 or more.

Given the range of an unsigned short, you were kind of lucky the error showed up so early in development. You may never have seen it, but it would have failed on a different machine, a different day, or with different compiler options, in the same way that on mine it worked.

Incidentally, it is always worth compiling your code with optimisation since the optimiser can detect errors not seen during a debug build. For example the following warning is emitted with -O1 -Wall:

main.cpp:39: warning: 'tmp' might be used uninitialized in this function

Let the compiler work for you! It does this because initialisation itself is often redundant if a variable is always assigned before use, so the optimiser, in the process of discovering whether a variable needs to be initialised at all can also determine that it is not when it should be.

You realise that using a fixed seed will generate the same result every time? I assume that is deliberate?

Clifford

[Bruno Calado]

Thank you very much. You really help me. With your help I solved the bug.

The fixed seed is not deliberate. I know that I need to use a time function, but I don't know how to use that.

[cpns]

> With your help I solved the bug.

Ok, I thought I solved the bug! ;-)

> I know that I need to use a time function, but I don't know how to use that.

You might use any function that produces a unique or unpredictable result. time() is often used since it generates a new unique value for every second of time.

Using a fixed seed is useful for tests that need to be repeatable (when debugging for example). If you are going to set it with a fixed value, you usually might as well never set it at all - calling srand() with a known value just changes which repeatable sequence you have.

Clifford