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;
}

• ratchet freak: You did it right. The "ask question" layout comes from the fact that this site is part of a Q&A network and this site is still using the default layout until the designers can get busy creating a custom one (and the powers that be decide this site is viable).

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.

• Jamal: main() doesn't require parameters per se. But yes, they are most recommended, especially if this will be run on a Unix system. Other than that minor thing, I agree with these things.
• Gerard: I have removed the item with regards to the function prototype of main(). The C++ standard (section 3.6.1.) specifies both (xor) uses are allowed.
• Edward: Uppercase names are not "reserved" for macros. That's merely a common convention.
• Gerard: @Edward Agreed. I have updated the corresponding item to reflect this.
• user110381: Also, He can just use the return type of the CreatePassword function to void.

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.)