#include <stdio.h>
#include <stdlib.h>
#include <time.h>
int main()
{
int i = 0;
int n = 0;
int randomizer = 0;
srand((unsigned int)(time(NULL)));
char numbers [] = "1234567890";
char letter [] = "abcdefghijklmnoqprstuvwyzx";
char letterr [] = "ABCDEFGHIJKLMNOQPRSTUYWVZX";
char symbols [] = "!@#$%^&*(){}[]:<>?,./";
printf("\nHow long password:");
scanf("%d", &n);
char password[n];
randomizer = rand() % 4;
for (i=0;i<n;i++)
{
if(randomizer == 1)
{
password[i] = numbers[rand() % 10];
randomizer = rand() % 4;
printf("%c", password[i]);
}
else if (randomizer == 2)
{
password[i] = symbols[rand() % 26];
randomizer = rand() % 4;
printf("%c", password[i]);
}
else if (randomizer == 3)
{
password[i] = letterr[rand() % 26];
randomizer = rand() % 4;
printf("%c", password[i]);
}
else
{
password[i] = letter[rand() % 21];
randomizer = rand() % 4;
printf("%c", password[i]);
}
}
return main();
}
#1 楼
我看到了一些我认为可以帮助您改进代码的东西。将程序分解为函数
这里的所有逻辑都放在一个相当长且密集的块中
main
代码。最好将其分解为单独的函数。检查返回值是否有错误
调用
scanf
可能会失败。您必须检查返回值,以确保没有返回值,否则当输入格式错误或由于系统资源不足时,程序可能崩溃(或更糟)。严格的错误处理是大多数正常运行的软件与无错误的软件之间的区别。您应该为后者而奋斗。使用更多的空格来提高代码的可读性
而不是像这样将事情挤在一起:
for (i=0;i<n;i++)
如果您使用更多的空间,大多数人会发现它更易于阅读:
for (i=0; i < n; i++)
消除“魔术数字”
与其对代码中的常数26和4进行硬编码,不如使用
#define
或const
并为其命名。请避免使用
scanf
那里
scanf
有很多众所周知的问题,通常最好避免这样做。 不要递归调用
main
可以在C中递归调用
main()
函数,但这不是一个好主意。您可能会炸毁堆栈,实际上没有充分的理由在这里这样做。只需使用一个循环。有关详细信息,请参见https://stackoverflow.com/questions/4238179/calling-main-in-main-in-c。使用更好的随机数生成器
当前正在使用
password[i] = numbers[rand() % 10];
这种方法存在许多问题。这将比较高的数字更经常产生较低的数字-这不是统一的分布。另一个问题是随机数发生器的低阶位不是特别随机,因此结果也不是。在我的机器上,有一个偏小但可测量的偏向0。有关详细信息,请参见此答案,但我建议将其更改为该链接中的
rand_lim
,并在下面进行重复。结果
这里是使用所有这些想法的替代方法。它还从命令行获取长度,因此不需要提示符或
scanf
。它消除了对字符计数的需要,并使用了上面提到的更好的随机数生成器:#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>
int rand_lim(int limit) {
/* return a random number between 0 and limit inclusive.
*/
int divisor = RAND_MAX/(limit+1);
int retval;
do {
retval = rand() / divisor;
} while (retval > limit);
return retval;
}
char picker(const char *charset) {
return charset[rand_lim(strlen(charset)-1)];
}
int main(int argc, char *argv[])
{
if (argc != 2) {
puts("Usage: pwgen len");
return 1;
}
int len = atoi(argv[1]);
if (len <= 0) {
puts("Length must be a positive non-zero integer");
return 2;
}
const char* groups[] = {
"1234567890", // numbers
"abcdefghijklmnoqprstuvwyzx", // lowercase
"ABCDEFGHIJKLMNOQPRSTUYWVZX", // uppercase
"!@#$%^&*(){}[]:<>?,./", // symbols
};
const size_t numGroups = sizeof(groups)/sizeof(groups[0]);
srand((unsigned int)(time(NULL)));
// only proceed if we got a number
for ( ; len; --len) {
unsigned group = rand_lim(numGroups-1);
putchar(picker(groups[group]));
}
putchar('\n');
}
评论
\ $ \ begingroup \ $
我喜欢您的大部分建议。但是我认为OP样本中的最大问题是使用不良的随机数生成器来生成“随机”密码。我对“使用更好的随机数生成器”部分提出建议,建议将rand()替换为rand()/10。指向getrandom()或RtlGenRandom()甚至是/ dev / urandom。在代码示例中对均匀性的处理是一个很好的考虑因素。在某些平台上,可以使用arc4random_uniform()函数生成统一范围内的数字,而无需滚动自己的数字。 (不幸的是,它不在Linux libc中。)
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19/12/12在18:02
\ $ \ begingroup \ $
@ConradMeyer:我选择考虑可移植性来解决该问题,因此rand_lim仅使用标准库函数。至于更大的问题,我将进一步讲,整个密码生成方法存在严重缺陷。
\ $ \ endgroup \ $
–爱德华
19年12月12日在18:17
\ $ \ begingroup \ $
C标准不需要实现提供真正的随机数生成器,但是您要编写这样的程序的任何真正实现都可以。可移植性通常是一个很好的愿望,但这并不意味着我们应该提供使用非随机输入的“更正”密码生成器代码的危险示例。人们确实确实从stackoverflow上复制/粘贴了代码,而这些人却是同一个人,他们认为随机数/随机数是随机的。无论如何,那只是我的意见。 :-)
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19年12月13日在19:38
\ $ \ begingroup \ $
您是否打算“不要使用scanf,因为它不可靠,最好使用atoi”是在开玩笑,还是偶然的?
\ $ \ endgroup \ $
–罗兰·伊利格(Roland Illig)
20年1月28日在7:52
#2 楼
此行中存在一个错误:password[i] = symbols[rand() % 26];
symbols
只有21个字符长,因此,当rand() % 26
大于21(并且rand() % 26
恰好是21 21,则在密码中放入一个空字节)。您的意思是让这26个与letter
搭配使用,而让21个与symbols
搭配使用。如果您避免使用“魔术”数字,则发生这种情况的可能性较小。如果改写此错误,则很难(或者至少更容易发现)该错误:
password[i] = symbols[rand() % (sizeof symbols - 1)]
当然,您也可以
#define
宏(如Edward的答案也建议),但是我仍然会使用sizeof
来定义它:如果数组的大小发生变化,将不会忘记更新计数。#3 楼
我不知道这是否只是学习使用字符串和数字的练习,或者您是否打算使用它来生成真实密码。但是,如果这样做,这行可能会成为一个严重的问题:srand((unsigned int)(time(NULL)));
由于您使用当前时间(以秒为单位)来植入伪随机数生成器,您已严重限制了可能的密码数量。如果我知道您是在哪一天运行此程序来生成密码的,则只需要测试86400个可能的密码。那是16位的熵。如果我能猜出小时,那么您可以减少到12位。如果我知道确切的时间,也许是因为您的邮件中包含加密文本,那么我也知道您的密码。
(
RAND_MAX
仅由标准保证至少为32767,这将确保对应于15位熵,但在我的机器上是2147483647,即31位熵。)评论
\ $ \ begingroup \ $
我同意!我认为这是最大的问题。 rand()看起来像一个随机数接口,但实际上不是。它甚至不是一个好的快速仿真生成器。但是请注意不要将rand()的范围与熵混淆。您的兰特范围为31位,但熵为0位。如果OP开发平台支持getrandom()或getentropy(),或者不打开/ dev / urandom,则OP将非常有用。 (或者选择Windows 10+上的任何安全CSPRNG API,它们都由相同的优质算法提供支持。)
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19/12/12在17:47
\ $ \ begingroup \ $
RAND_MAX与随机数生成器的熵无关。仅将srand_bytes(const char *,size_t)添加到API即可改善熵,而无需触及RAND_MAX。这意味着您的论点是有缺陷的。 RAND_MAX仅描述对rand()的单个调用的熵,为此,15位的熵就足够了。
\ $ \ endgroup \ $
–罗兰·伊利格(Roland Illig)
20 Jan 28'在7:57
\ $ \ begingroup \ $
@RolandIllig:是的,我同意RAND_MAX与生成的密码的熵无关。
\ $ \ endgroup \ $
–托马斯·帕德隆·麦卡锡
20年1月28日在8:01
#4 楼
从每个分支中提取此行并将其放在循环的末尾:printf("%c", password[i]);
从每个分支中提取此行并将其放在循环的开始(并从循环之前将其删除):
randomizer = rand() % 4;
如Schwern观察到的,您还可以转动
if
链接到一个switch
/ case
,它更快,更容易阅读。这使循环看起来像:
for (i=0;i<n;i++)
{
randomizer = rand() % 4;
switch (randomizer) {
case 1:
password[i] = numbers[rand() % 10];
break;
case 2:
password[i] = symbols[rand() % 26];
break;
case 3:
password[i] = letterr[rand() % 26];
break;
default:
password[i] = letter[rand() % 21];
break;
}
printf("%c", password[i]);
}
评论
\ $ \ begingroup \ $
为什么停下来? struct {char * const str; size_t n; } table [] = {{数字,sizeof数字-1},{符号,sizeof符号-1},...},* t; for(i = 0; i
\ $ \ endgroup \ $
–尼尔
19/12/9在22:07
#5 楼
这是一个好的开始!但是您确实有一些错误。首先,这两行:
password[i] = symbols[rand() % 26];
和
password[i] = letter[rand() % 21];
似乎混淆了它们的编号-swap
21
和26
在这里。第二,这行是不好的做法。它可能有效,但不是执行此操作的“正确”方法。如果他们看到了,我的编码老师会有点his!
char password[n];
我被告知,变量声明应该几乎总是在函数的顶部。解决此问题的“正确”方法是简单地声明一个char指针,然后在知道字符串的大小时使用
malloc
向该字符串分配正确的内存量,并且不要忘记在不再使用该字符串时释放该内存char *password = NULL;
printf("\nHow long password:");
scanf("%d", &n);
password = (char *)malloc((n + 1) * sizeof(char)); /* String length +1 for NULL terminator */
...
free(password);
第三,清理输入内容-始终假设最终用户是世界上最大的白痴,当要求输入
How long password:
时,只需输入twelve
作为输入即可。回答,甚至只是cabbage
。一个简单的循环可以做到这一点,对提示的微小更改可以帮助减少使程序崩溃的无效答案:第四,我个人将使用
switch
/ case
而不是if
/ else
if
/ else
,但这只是个人喜好。最后,我只能同意Edward的回答。
这里是您要进行更改的代码:
do
{
printf("\nHow long password (8-32):");
scanf("%d", &n);
} while ((n < 8) || (n > 32));
评论
\ $ \ begingroup \ $
我被告知,变量声明应该几乎总是在函数的顶部。我不同意;变量必须在初始化之前并在尽可能小的范围内进行声明,以免意外地从常用的变量名(如i和n)中“泄漏”值,并增加在执行时出现编译器错误或警告的机会错误。当必须在函数顶部声明所有变量时,“始终将变量放在函数顶部”是C89的保留。今天,这是个坏建议。
\ $ \ endgroup \ $
– trentcl
19/12/10在12:58
\ $ \ begingroup \ $
另外,虽然我在这里也会使用malloc,但这并不比使用VLA(可变长度数组)更正确。您的老师可能很健康,但这只是他们的见解,而不是福音的真理。福音的真理是void main()是非标准的。
\ $ \ endgroup \ $
– trentcl
19-12年10月10日在13:02
\ $ \ begingroup \ $
@StefanCole“有人告诉我,变量声明应该几乎总是在函数的顶部。” C89的日子已经过去很久了。
\ $ \ endgroup \ $
–亚历山大
19/12/10在22:07
\ $ \ begingroup \ $
rand()%21我认为这会导致模偏差,因为RAND_MAX不能被21整除。
\ $ \ endgroup \ $
–亚历山大
19/12/10在22:08
\ $ \ begingroup \ $
C89要求在作用域块的开始处定义变量,该作用域比函数的开始处要宽。
\ $ \ endgroup \ $
–太糟糕了
19年12月11日上午11:17
#6 楼
不要用字符串初始化动态字符数组。即对于一个(隐式)自动变量,请不要这样写:
char numbers [] = "1234567890";
如所写,该数组在运行时从常量复制到自动变量。 br而不是这样写:
char *numbers = "1234567890";
或者更好的方法是:
char const *const numbers = "1234567890";
或者将其移到函数以使其成为全局函数,或将其声明为静态函数(在这种情况下,数组在编译和加载时初始化)。因此,编写类似以下内容也是合理的:
static char const numbers[] = "1234567890";
不要使用
rand()
和srand()
。他们真的只是老去了。至少使用rand48()
和srand48()
。更好的是,找到一个加密级随机数生成器并使用它。这与前面提到的“请勿使用scanf()
”一起使用。 (还有其他一些您不应该使用的功能,例如gets()
。)如果您尝试更全面地开发此功能,则可能会开始受到额外的限制,例如不要同时使用“ 0”和“ O” ,或同时为“ 1”和“ l”。这两个是关于消除混乱字符的。另一个可能是更改符号集,因为许多站点使用受限符号集。 (而且我注意到您自己已经消除了“,',`和;;)。因此,我建议动态建立一个可接受字符的单个列表,并为该列表使用动态长度。
我个人总是就像通过生成加密级别的随机字符(即8位样本)然后丢弃不可接受的(不可打印的)字符来生成密码。我敢肯定有一些原因导致这种情况不好。
评论
\ $ \ begingroup \ $
为什么不使用静态const char number [] =“ 1234567890”;里面的功能?
\ $ \ endgroup \ $
–马丁·邦纳(Martin Bonner)支持莫妮卡(Monica)
19/12/11在17:43
\ $ \ begingroup \ $
@Martin:在函数内部或外部都可以。关键是要初始化它的编译/加载时间,而不要初始化运行时间。我确实说过“使其成为全局的,或者声明为静态的”。尽管在全球范围内静态的语义是不同的,但当然可以做到这两者。
\ $ \ endgroup \ $
– David G.
19/12/11在19:38
\ $ \ begingroup \ $
4)很好,只要您简单地删除不可打印的字符,然后重复直到您有足够长的字符串即可。这是获取无偏模N随机数的标准方法之一。 JDK以这种方式进行操作(循环执行,直到得到的数字小于不能无偏向不同存储桶的最大映射数为止)。
\ $ \ endgroup \ $
– Voo
19年12月12日在17:21
\ $ \ begingroup \ $
我建议您避免使用任何非加密的非种子libc PRNG,包括rand48。只需直接跳到getrandom(),或者如果您使用的是较旧的平台,/ dev / urandom,或者是Windows(尤其是10+),那里提供了不错的CSPRNG API,我不记住以下名称: )。
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19年12月12日在17:51
#7 楼
以前的评论者暗示了这样一个事实,即编程语言库提供的rand()函数充其量是伪随机的,并且不能提供安全的密码。在类似Unix的系统上使用/ dev / random设备(https://en.wikipedia.org/wiki//dev/random)是合理的,而在Windows系统上使用Microsoft库CryptGenRandom(https:// en .wikipedia.org / wiki / CryptGenRandom-此处的轻量级替代方法:https://blogs.msdn.microsoft.com/michael_howard/2005/01/14/cryptographically-secure-random-number-on-windows-without-使用-cryptoapi /)作为随机性源,因为它们使用观察到的外部事件(例如,来自设备驱动程序)来生成随机数池,而不是使用可预测其输出的伪随机数生成器。评论
\ $ \ begingroup \ $
嗨,埃里克!我完全同意在这里使用兰特,并同意您建议的替代产品。不过,有一种语言在bble测:兰德并不是“最好的伪随机”。根据C标准,明确要求它是确定性序列生成器。
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19/12/12在17:55
\ $ \ begingroup \ $
好点。我的评论“最好是伪随机的”是对rand()实现本身可能存在缺陷的真正担忧。别笑-这里有一些很好的例子。
\ $ \ endgroup \ $
–埃里克·科斯基(Eric Koski)
19/12/12在20:57
\ $ \ begingroup \ $
啊,当然。 “九,九,九,九...”
\ $ \ endgroup \ $
–康拉德·迈耶(Conrad Meyer)
19年12月13日在19:30
#8 楼
返回到已经提到的递归主逻辑。如果经常运行程序逻辑,则会生成堆栈溢出并耗尽内存。
您可以请使用永久循环:
int main()
{
for(;;) {
// youre code here
}
return 0;
}
尚未提及的改用
for(;;)
的另一个优势:这样,您还可以在
for(;;)
之前重构一些东西,例如常量。评论
\ $ \ begingroup \ $
这是一个很好的观点,但是我确实在回答中提到了这一点。
\ $ \ endgroup \ $
–爱德华
19年12月11日在17:11
\ $ \ begingroup \ $
你说得对,我读完了。
\ $ \ endgroup \ $
– Sandro4912
19/12/11在17:12
评论
这是Unicode密码的很小一部分。顺便说一句,实际上,不要将由此生成的密码用于任何事情–超出Edward提到的偏见,rand()在大多数平台上都是弱的,可预测的并且可逆的。
您应平等对待所有字符,否则会降低分布的随机性。使用当前的方法,给定数字字符的可能性是任何非数字字符P(“ 1”)= 1/4 * 1/10 = 1/40,P(“ A”)= 1/4的两倍以上* 1/26 = 1/104
大写字母的可变名称字母似乎很懒
这种分配不需要使用每个组中的某些内容,那么为什么还要有组呢?即使用“ 1234567890abc…xyzABC…XYZ!@#$%^&*(){} []:<>?,。/”,其他大多数代码将折叠成一个小循环。 —或者更好的是,假设使用ASCII,只需选择介于33和126之间的随机整数并将其用作字符,甚至不需要字符列表。