Console random password generator

Question

This program is a random password generator. It asks the user how many chars they want their password to be and how many passwords it should generate. I used the rand() and srand() methods to generate random letters.

I used dynamic allocation to create a char array based on user input. The comment line is where I tried to dynamically allocate an array of pointers, where each pointer would point to a char array, but I couldn't figure out how to do it so I used a counter and a do while loop and a for loop to create passwords based on user input.

Please give me advice on how I can improve or what I can do differently!

#include <iostream>
#include <string>
#include <cstdlib>
#include <ctime>

const int MAX = 90;
const int MIN = 65;

char * createPassword();

int main()
{
    char * p = createPassword();    




    return 0;
}

char * createPassword()
{
    unsigned seed = time(0);

    srand(seed);

    char x = ' ';
    int passwordLength = 0;
    int numOfPasswords = 0;

    std::cout << "How many chars in password: ";
    std::cin >> passwordLength;
    char * pwptr = new char[passwordLength];

    std::cout << "How many passwords should be generated?";
    std::cin >> numOfPasswords;
    //char * passwords = new char *pwptr[numOfPasswords];

    int passwordcount = 0;

    do{
        for(int cnt = 0; cnt < passwordLength; cnt++)
        {
            x = (rand() % (MAX - MIN + 1)) + MIN;
            pwptr[cnt] = x;
            std::cout << pwptr[cnt];
        }
        std::cout << std::endl;
        passwordcount++;
        } while(passwordcount != numOfPasswords);

        return pwptr;
}

Answer 1

A few things immediately jump out:

1. Use std::string instead of char*.
2. Your string is not null terminated (use of std::string fixes this)
3. Don't create two globals when they're only used in one place.
4. Don't create globals with an "uppercase" name. By common practice this is reserved for macros.
5. srand() and rand() are superseded by any RNG from the <random> header.
6. Don't use a do...while when a for-loop does the same job more succinctly.

7. Your createPassword function does two things:

   1. It communicates with the user.
   2. It creates a new password.

   Split the logic into 2 functions.

Answer 2

Coding style has already been addressed. I would like to add a note about randomness, because the same pitfalls exist even when using modern <random> utilities.

x = (rand() % (MAX - MIN + 1)) + MIN;

The C library function int rand(void) returns a pseudo-random number in the range of 0 to RAND_MAX

This results in a skewed distribution. Suppose RAND_MAX == 32767 and MAX - MIN + 1 == 100. rand() produces a more or less uniform distribution.

What is the probability of rand() % 100 == 67? It's (32767 / 100) + 1.

What is the probability of rand() % 100 == 68? It's (32767 / 100).

Answer 3

The​ numbers 65 and 90 are magic numbers, since at their definition there is no hint that they refer to Unicode code points. Instead of these numbers, write 'A' and 'Z'. Then, hope that your program will never run on IBM machines, since your passwords would contain characters other than uppercase letters. (See EBCDIC for the details.)