Password generator with multiple charset

Question

This is just straight bruteforce with the ability to select multiple charset for one word, being able to mash up the linearity with a modulo. This code is also posted on GitHub.

Generateur.h

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

class Generateur
{
private:
    bool two, three;
    int length1, length2, length3, min, max, loop, size3;
    char *first, *middle, *last, *tmp, *ptr;
    long counter2, counter3, *array;
    void exiterr(char *msg);
    void gen(long index0);

public:
    Generateur(int argc, char *argv[]);
    ~Generateur();
};

zhou.cpp

#include "Generateur.h"

int main(int argc, char *argv[])
{
    Generateur gen(argc, argv); 
}

Generateur.cpp

#include "Generateur.h"

Generateur::Generateur(int argc, char *argv[])
{
    if  (argc < 4)              { printf("./zhou min max [first] middle [last [length]].\n");exit(-1); }
    if(!(min = atoi(argv[1])))  { printf("./zhou min max [first] middle [last [length]].\n");exit(-1); }
    if(!(max = atoi(argv[2])))  { printf("./zhou min max [first] middle [last [length]].\n");exit(-1); }

    long index0;//this one runs
    two = three = false;
    index0 = loop = 0; //index is the core variable with the array, loop is needed in case max > min
    counter2 = counter3 = 0;
    size3 = 1; //when not used we need it anyway because we are substracting it to the password's length for looping 

    array = new long[max];
    tmp = new char[max];
    first = new char[strlen(argv[3])];
    strcpy(first, argv[3]);
    length1 = strlen(first);

    if(argc > 4)//is there a second charset ?
    {
        two = true;
        middle = new char[strlen(argv[4])];
        strcpy(middle, argv[4]);
        length2 = strlen(middle);
    }
    if(argc > 5)//a third ?
    {
        if(argc < 7)    { printf("Needs expand of last character set\n"); exit(-1); }
        three   = true;
        last    = new char[strlen(argv[5])];
        strcpy(last, argv[5]);
        length3 = strlen(last);
        size3 = atoi(argv[6]);
    }
    for(int i=0; i<max; i++)
    {
        array[i] = 0;   
    }
    gen(index0);//Entry
}

Generateur::~Generateur()
{
    delete(tmp);
    delete(first);
    if(two) 
        delete(middle);
    if(three)
        delete(last);
}

void Generateur::gen(long index0)
{
    if(index0 == 0)
    {
        for(loop=0; loop<max-min+1; loop++)
            for(array[index0]=0; array[index0]<length1; array[index0]++)
                if(index0 < min+loop-1) gen(index0+1);
    }
    else
    {
        for(array[index0]=0; array[index0]<length1; array[index0]++)
        {
            if(index0 < min+loop-1 && !two) gen(index0+1);
            else
            {
                ptr = &tmp[0];
                for(int a=0; a < min+loop; a++)
                {
                    snprintf(ptr, 2, "%c", first[(array[a]+counter3)%length1]);
                    ptr++;
                }
                if(two)
                {
                    for(array[index0]; array[index0]<length2; array[index0]++)
                    {
                        if(index0 < min+loop-size3) gen(index0+1);
                        else
                        {   
                            ptr = &tmp[1];
                            for(int a=0; a < min+loop-size3; a++)
                            {
                                snprintf(ptr, 2, "%c", middle[(array[a+1]+counter2)%length2]);
                                ptr++;                          
                            }
                            if(three)
                            {
                                for(array[index0]; array[index0]<length3; array[index0]++)
                                {
                                    if(index0 < min+loop-1) gen(index0+1);
                                    else
                                    {
                                        ptr = &tmp[min+loop-size3];
                                        for(int a=0; a<size3; a++)
                                        {
                                            snprintf(ptr, 2, "%c", last[(array[min+loop-size3+a])%length3]);
                                            ptr++;
                                        }
                                        printf("%s\n", tmp);
                                        counter3++;
                                    }
                                }
                            }
                            else
                                printf("%s\n", tmp);
                            counter2++;
                        }
                    }
                }
                else
                    printf("%s\n", tmp);
            }
        }
    }
}

Answer 1

This is very much C style code. You could likely extract the functionality out of your Generateur class into reusable functions and the reclassify this as C.

If you wanted to keep this in C++ I would completely rewrite this.

Start with your headers. Use the C++ versions of C Library headers. They are prefixed with c and don't end in .h:

#include <cstdlib>
#include <cstdio>
#include <string>

Also use conventional C++ constructs. C++ has RAII containers that handle memory management for you. It has iterators as well so you don't need to maintain raw pointers.

I/O is typically done with <iostream>

You would also benefit from not declaring multiple variables on a single line. It is error prone and difficult to read.

It is also customary in C++ to put the type specifier with the type not the variable. This is part of the reason that doing multiple declarations on a single line can be error prone.

char *first, *middle, *last, *tmp, *ptr;

would look like this:

char* first;
char* middle;
char* last;
char* tmp;
char* ptr;

You would also benefit from the use of more whitespace around your operators. It's simply more readable. Also prefer prefix increment/decrement to postfix. Postfix returns an unmodified copy, so unless you need the copy don't use it.

for(int a = 0; a < (min + loop); ++a)

class is private by default. It is however conventional to list public members and functions first and then declare private members and functions. The reason is anyone (including you) using your class needs to know what publicly accessibly parts they have available. The implementation details however rarely matter (once you get them working right).

You should also make the class reusable. your Generator runs on construction but there is no public function to use it again. Once it's used it can't be used again. might as well make it a function then. Make a callable function like generate_pass() and have it make a new password for you. You can have the function accept input if needed. perhaps seed it from a PRNG.

You should also initialize member variables in your class whenever possible. Assignment in the constructor is not the same thing. There are a couple different methods for this but I will just start with brace initialization because that is the most preferred where possible. It looks something like this:

private:
    bool two{ false };
    bool three{ false };

Answer 2

Your functions are so long, and do so much things.

Yeah, sometimes, the size matter. Although, there is no true limit number of lines, usually, a too long function expose a poor design.

One function (or method) have to do one thing, and only one. If this "one thing" can broken into smaller things, each of those should be placed into a separated function. It's a kind of "Single Responsability Principle".

Perfection is finally attained not when there is no longer anything to add, but when there is no longer anything to take away. - A. de Saint Exupéry

Eg, talking about me, I almost always try to don't excess 30-35 lines of codes for a function, because it force me to separate logic units. With a short function, you can immediately determine it purpose.

A shorter function, by definition (most of the time), is simpler to understand. It's also easier to debug if you get a problem later. It's the "KISS principle"

Everything should be made as simple as possible, but not simpler. - A. Einstein

Therefore, a good practice to following is to avoid deep nesting because it force you to keep like a "stack trace" in mind and overload your brain. In Generateur::gen you have 21 structure of control and the deepest is at the 11th level. Don't you think it's too much?

---

Also, in many place, you repeat same code, again and again. So would be to try to factorize your code, applying the Don't Repeat Yourself principle. This one make reading, writing, editing of your code easier and faster, since we only have to read, write or modify once.

That also prevent your code to being a huge "copy and past" fest. So, instead of hard-coding three times how using your program, write a ´usage` function where you print that, all call it from where you want.

For your arguments parsing, you can try to code a tiny reusable utility for parsing command lines or rely on an already existing solution.

---

A lot of your code can be simplified using Standards C++ classes, but I will not go into too much detail about what should be changed in your code because I think it's all the code that needs to be rewritten taking into account everything I've said. In summary:

• Make the functions shorter
• Less use of conditional branching

• Factorize into pieces of reusable code

• Don't use old C Library header when you have Standard C++ features that allow you to do it easier, better, faster, stronger and safer (Hello Daft Punk). In last resort, if you have to use C Library, use the <cXXX>version instead of the <XXX.h> one.

• Use the language facilities offered by the STL and C ++ in general.

• Your arguments parsing can (and should) be rewrites with: std::string, std::vector and std::optional.

• Use std::stoi (and his family) , std::stringstream or the C++17 std::from_chars instead of the old C atoi.

• Use std::string first = argv[3]; instead of char *first; first = new char[strlen(argv[3])]; strcpy(first, argv[3]); length1 = strlen(first);
• Learn C++ algorithms

As a final word, here is a quote from Bjarne Stroustrup, the father of C++

Using Standard C++ as a glorified C or glorified C with Classes only would be to waste the opportunities offered by Standard C++. - Bjarne Stroustrup (Learning Standard C++ as a New Language)

Answer 3

Do arg parsing in main and pass the result of that to the constructor of Generateur.

Use std::vector<long> instead of the new long[] array. Use std::string or std::vector<char> instead of the new[]ed char arrays.

This lets you remove the destructor because the fields of Generateur will take care of leaks themselves. As an aside when you use new[] you must use delete[] instead of plain delete.

snprintf(ptr, 2, "%c", middle[(array[a+1]+counter2)%length2]);

can be replaced with:

ptr[0] = middle[(array[a+1]+counter2)%length2];
ptr[1] = '\0';

however adding the null terminator only matters when printing it out.