[C]Problem in creating a random string

xXxRevolutionxXx

Hello ! I just started to play around with C lately(and mainly with pointers). For practice, i created this function that is suppose to create a random string:

char *ReturnRandomString(int Range)
{

char StringsToChooseFrom\[26\] = {'q','w','e','r','t','y','u','i','o','p','a','s','d','f','g','h','j','k','l','z','x','c','v','b','n','m'};
char MyArray\[Range + 1\];
char \*Ptr\_str;
int i;

for(i = 0; i < Range; i++)
{
    srand(time(NULL));
    int RandomInt = rand() & 26;

    MyArray\[i\] = StringsToChooseFrom\[RandomInt -1\];
}
MyArray\[i + 1\] = '\\0';

Ptr\_str = malloc(sizeof(MyArray));
Ptr\_str = MyArray;
return(Ptr\_str);

}

I am "calling" this function this way:

char *PtrCharArray = ReturnRandomString(5);
printf("\nString Array returned: %s", PtrCharArray);

The output... is nothing. I don't neither an output on the console... either an error. It's sure that i am missing something, but i don't know what. Thanks in advance !!! P.S No hate.

Lost User

char MyArray[Range + 1]; // 1

// ...
Ptr_str = malloc(sizeof(MyArray)); // 2
Ptr_str = MyArray; // 3
return(Ptr_str);

That won't work: 1. You cannot allocate an array on the stack using a variable value. 2. sizeof is compile time so it cannot calculate the actual size of the array. You need to use strlen. 3. You cannot use an assignment to copy data. You need to use a copy function: memcpy or strcpy MyArray to Ptr_str.

Jochen Arndt

There are some more errors besides those mentioned by Richard. Here you would normally use the % operator so that the value is in the range 0 to 25:

int RandomInt = rand() & 26;

Anding with 26 (0x1A) will clear masked out bits so that the possible values are only 0x00, 0x02, 0x08, 0x0A, 0x12, 0x18, and 0x1A. Here you should remove the subtraction of one because that accesses the array out of bounds when RandomInt is zero:

MyArray[i] = StringsToChooseFrom[RandomInt -1];

See the rand() description:

Quote:

The rand() function returns a pseudo-random integer in the range 0 to RAND_MAX inclusive.

leon de boer

What Jochem said .. RTM and look at the examples you need to use the MODULO trick Can you also please put this line up before the i loop

srand(time(NULL));

That just sets the random seed to the current time tick which updates every second again RTM on C standard time function. I assure you that loop will complete far faster than 1 second on any decent computer so you are wasting time setting the same seed i times. You might as well just do it once before the loop :-)

In vino veritas

k5054

1. You cannot allocate an array on the stack using a variable value.

C99 and C11 both permit this. Additionally, both gcc and clang will accept this when compiling in C89 mode, too. If you're using a POSIX system, then rather than doing malloc() and strcpy() then perhaps use strdup() instead. MSVC has _strdup(), which does the same thing.

David Crow

In addition to the other comments and suggestions:

_TCHAR *ReturnRandomString( int Range )
{
_TCHAR *Ptr_str = (_TCHAR *) malloc(Range + 1);
int i;

for (i = 0; i < Range; i++)
{
    int RandomInt = rand() % 26;

    Ptr\_str\[i\] = (\_TCHAR) (RandomInt + 97);
}

Ptr\_str\[i\] = \_T('\\0');

return Ptr\_str;

}
...
srand(time(NULL));

_TCHAR *PtrCharArray = ReturnRandomString(5);
_tprintf(_T("String Array returned: %s\n"), PtrCharArray);
free(PtrCharArray);

"One man's wage rise is another man's price increase." - Harold Wilson

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons

"You can easily judge the character of a man by how he treats those who can do nothing for him." - James D. Miles

Lost User

Thank you. Actually I did think that was the case, but when I tried to compile it the compiler objected, so I assumed I just mis-remembered.

Daniel Pfeffer

_TCHAR *Ptr_str = (_TCHAR *) malloc(Range + 1);

This line only allocates the correct amount of memory if sizeof(_TCHAR) == 1. The correct code would multiply the length by sizeof(_TCHAR).

Ptr_str[i] = (_TCHAR) (RandomInt + 97);

This line only works on character sets that encode the alphabet consecutively. This is true for ASCII and Unicode, but not for all varieties of EBCDIC, for example. A portable solution would be as follows:

static _TCHAR const chars[] = _T("abcdefghijklmnopqrstuvwxyz");

TCHAR *Ptr_str = (_TCHAR *) malloc((sizeof(_TCHAR)*(Range + 1));

...
Ptr_str[i] = chars[RandomInt];

Where _TCHAR and _T(x) have appropriate definitions for the environment.

If you have an important point to make, don't try to be subtle or clever. Use a pile driver. Hit the point once. Then come back and hit it again. Then hit it a third time - a tremendous whack. --Winston Churchill

Lost User

May this will help...

char *ReturnRandomString(int Range)
{

char StringsToChooseFrom\[26\] = {'q','w','e','r','t','y','u','i','o','p','a','s','d','f','g','h','j','k','l','z','x','c','v','b','n','m'};
char MyArray\[100\] = {}; // char MyArray\[Range + 1\];
char \*Ptr\_str;
int i;

for(i = 0; i < Range; i++)
{
    int RandomInt = rand() % 26; // int RandomInt = rand() & 26;

    MyArray\[i\] = StringsToChooseFrom\[RandomInt\];
}
MyArray\[i + 1\] = '\\0';

Ptr\_str = (char\*)malloc(sizeof(MyArray)); // Ptr\_str = malloc(sizeof(MyArray));
strcpy(Ptr\_str, MyArray ); //	Ptr\_str = MyArray;
return(Ptr\_str);

}

int _tmain(int argc, _TCHAR* argv[])
{

srand(time(NULL)); // Moved out from function.
for( int i = 0; i < 10; i++ )
{
    char \*PtrCharArray = ReturnRandomString(i);
    printf("\\nString Array returned: %s\\n", PtrCharArray);
}
return 0;

}

k5054

Only as long as Range < 100. Which also brings up the question of what happens if Range < 0. It might be better to use size_t for Range and your loop variable i. You should probably check the return value of malloc() too, just to be safe. Oh. And you're leaking the results of ReturnRandomString(). There should probably be a free() when you are done with it.

Lost User

Thumps up!!!

David Crow

Krishnakumartg wrote:

MyArray[i + 1] = '\0';

Why add 1? When the loop ends, i is already 1 past the desired length.

"One man's wage rise is another man's price increase." - Harold Wilson

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons

"You can easily judge the character of a man by how he treats those who can do nothing for him." - James D. Miles