#CLI Password Generator

27 messages · Page 1 of 1 (latest)

silent notch
lapis lichen
#

why paste bin instead of github 👀

primal plume
#

If too few arguments would output usage instruction (which is common for CLI tools that don't work without arguments), that would not only help user, but also document conditional code that works with arguments.
Code style could be improved like magic number 4 changed to constant with meaningful name.
this is allowed: int i = 'c';
so can use characters instead of writing character codes, should be much more readable.
triple insert calls repeated several times with same arguments should be organized better like put into function, so that there is no such copy-paste code.
If there is logical minimum length there likely should be logical maximum length too, something smaller than maximum int.

Clean clipboard after certain timeout. For Windows, that would be powershell script that calls generator, exits if fail or waits for a short time, then cleans clipboard.

#

Maybe something else.

silent notch
silent notch
# primal plume If too few arguments would output usage instruction (which is common for CLI too...

Thank you for your comments, I will revise the code.

this is allowed: int i = 'c';
yes but there is no 'ä' or '€' for example, so i used the codes so that everything is consistent.
I have a function that I could use to do a UTF-8 decode, i.e. turn a string into a vector of grapheme code points, but that would be unnecessary extra work that slows down the code just to make it look nicer, which I didn't want.
Would it be better to write all ASCII characters as char and then only use the code point as int for the non-ASCII Unicode graphemes?

If there is logical minimum length there likely should be logical maximum length too, something smaller than maximum int.
the minimum length is 1, i just print a warning when the length is < 16, because that is unsecure, but i do not see why i should have a maximum lower than INT_MAX?
i mean sure no one needs such a long password, but why should my generator not be able to generate this length?

sullen aurora
#

  1. you can define a copyToClipboard function in a separate header file and use preprocessor directives like #ifdef WIN32 ... #elif defined (_linux) ... to provide a windows or linux implementation of the copy depending on the platform and then just use #include "copy_to_clipboard.h" everywhere

  2. codepointToUTF8 has an unnecessary const for it's input parameter and really should have a lot of comments explaining what each line does because it's hard to reason what exactly it's doing and why you chose such specific values like 0x3F bitwise and with codepoint and then bitwise or with 0x80

  3. are you really sure about the order in which bitwise operations are performed in the codepointToUTF8 function? these are easy to messup, it would be better if you put parenthesis to clear that up for anybody reading

  4. in generate function it's not immediately clear what would happen if size was less than 4 and what the output in this case should be, in the end what is "size" supposed to represents? it's not really clear what it is the size of. also note again the unnecessary const in the lambda passed to std::ranges::for_each

  5. the function getPasswordSize as far as I can see is supposed to read an integer passed from console and turn it into an int? first it has an unnecessary const & for it's input, then it uses a weird way to parse an integer from a character array using std::from_chars when you can use std::stoi and exceptions it throws to provide an extra information to the user instead of writing to a std::err and exiting the program - the function that returns password size shouldn't be responsible for writing to console and exiting the program

  6. I see only one use of copyToClipboard in makePassword function which means you can safely replace the input type of copyToClipboard with const std::string& (or even better std::string_view) instead of const char*

#

  1. in main function as has been said I'd move all the vector initializations to separate functions, I would use std::iota because you have a clear continuous range of values in some vectors and use vector constructor to fill it with zeros initially for iota, then return each vector by value, while also adding a comment what each vector represents because the names "ALPHA_LOW", "ALPHA_HIGH_ASCII", "SPECIAL" or "NUMS" are really hard to interpret

  2. all these vectors are default values built into a program, so consider making a function that would initialize a struct that would contain all these vectors and then pass that struct to other functions instead of passing a ton of vectors to each

  3. you can avoid extra indentation in the if (argc == 3) statement by negating it as if (argc != 3) return EXIT_FAILURE;

  4. a lot of repetition happens in checking the console arguments so as been said earlier consider somehow turning it into a couple of functions as well

silent notch
#

you can define a copyToClipboard function in a separate header file and use preprocessor directives like #ifdef _WIN32 ... #elif defined (linux) ... to provide a windows or linux implementation of the copy depending on the platform and then just use #include "copy_to_clipboard.h" everywhere

i still learn C++ so i focus on Windows only and WinApi for now and do not care about crossplattform, except it is really trivial

codepointToUTF8 has an unnecessary const

My IDE, gave me a grey underline saying ... can be made const so i did that because i thought it would be good practise to make "effectively const" variable so variables i never change, const, if the IDE already notes this. Is this wrong?

are you really sure about the order in which bitwise operations are performed in the codepointToUTF8 function? these are easy to messup, it would be better if you put parenthesis to clear that up for anybody reading

When i wrote this i put parenthesis but my IDE greyed them out and marked that they are redundant, that is why i removed them

the function getPasswordSize as far as I can see is supposed to read an integer passed from console and turn it into an int? first it has an unnecessary const & for it's input, then it uses a weird way to parse an integer from a character array using std::from_chars when you can use std::stoi and exceptions it throws to provide an extra information to the user instead of writing to a std::err and exiting the program - the function that returns password size shouldn't be responsible for writing to console and exiting the program

But std::from_chars is more modern than std::stoi and does not have the overhead of first constructing a std::string out of the char*, since the command line argument is a char*, and it is more performant anyways i think.
and i do not have to catch the error from std::stoi, but i check for the error

#

Thank you very much for your comments

sullen aurora
#

what IDE are you using? I've never seen an IDE suggest making an input parameter const, the issue with making input parameters const is that from the user perspective the input parameter is const regardless because it's going to be copied inside the function and if the function changes it internally the "external" copy of the parameter would not change regardless of whether it is marked const or not

silent notch
#

CLion

sullen aurora
#

I see. that might be the reason, I only used visual studio, Qt Creator and visual studio code, so CLion might have some differences

silent notch
#

i think CLion says can be made const for everything that you never change

#

i mean CLion uses clangd

#

so clangd says this

sullen aurora
#

maybe the have some sort of default config for clangd or something which does this

#

the use case is rare, but you sometimes want to internally modify a parameter that was captured by copy into the function, if it was const you'd have to create a separate non-const copy of it inside and use that

silent notch
#

should i make it a const int &cp to avoid copying?

sullen aurora
#

as for "from_chars", sure you can value performance in constructing the temporary string, but since in exceptional situations you're closing the program anyway by calling exit(1) you're not really loosing any performance in case of exceptions being thrown - you can even do a try catch block inside the function itself and then just throw a new error with a custom message which will automatically close the program and print the message to console and completely remove 4 if statements

sullen aurora
# silent notch should i make it a `const int &cp` to avoid copying?

no, that's even worse because references internally are implemented as simply compiler-checked pointers with a few special additions, every time a function has to access a small object like int by a pointer the CPU has to read that int from RAM when it could've been copied to registers (the fastest memory in CPU) with no RAM interactions required later

#

you can see this on e. g. godbolt where the instructions generated for copying and const& version are a bit different, the same is true for small enough structs and primitive types as well

2025-09-06_16-58-51.png
silent notch
silent notch
#

i thought copying is always worse than just taking it, but true the referencing has to have an overhead

silent notch
sullen aurora
#

looks good, though you don't need the constructor for this struct since you can (and already do) initialize it using the curly braces syntax