Is this C any good?

[ajft]

How very hard it is to tell the difference between someone honestly asking for help, and someone trying to get their homework done for them....

I'll merely suggest a C group for C questions, find lots of well written C and read it, and turn on all the warnings that your compiler has, read the warnings, understand the warnings, and correct the warnings if warranted.

[kremit]

you should malloc an extra byte and set the last byte of the buffer to 0 to null terminate the string.

[jolilly]

The code looks good, but one thing to remember is that in C, character strings are NULL terminated (\0). The printf ("%s\n", buffer) statement expects a NULL terminator in buffer to delimit the end of the character string (i.e. password).

I do not see that you are setting a NULL terminator after the password characters are generated (to indicate the end of the string), so a core dump could occur of buffer does not happen to be initialized to zeros. One approach you could take is to call calloc instead of malloc to initialize the buffer to zero before using it.

Also, you will need to allocate "buffer" to be one char larger than your password size (bytes) to accommodate the null terminator at the end of the string.

[geirha]

Don't assume that the user will enter correct input. Rather than use scanf(), read in a line with fgets() and convert it to an integer with sscanf(). Check return values for input or format errors. See manpages for details.

Or even better, take them in as command-line arguments.


@bobbocanfly
You need to seed the random number generator with srand(). A common way is to use time() (in <time.h>)

    srand( time(NULL) );

And also, the man-page for rand(3) states:

          "If  you  want  to  generate a random integer between 1 and 10, you
           should always do it by using high-order bits, as in

               j = 1 + (int) (10.0 * (rand() / (RAND_MAX + 1.0)));

           and never by anything resembling

               j = 1 + (rand() % 10);

           (which uses lower-order bits)."

[bobbocanfly]

Thanks for your feedback everyone. Was laughing at the person who thought this was homework help, I only with i was learning C at school. Currently we are 'learning' how to make websites in HTML, complete with <font> tag.

Also, to the person who suggested Python, thanks but as you said this is just a practise in learning C. I already have a Python password generator script (which interestingly enough ran just 0.03 seconds slower than my C code when generating 100, 1000 char passwords).

Anyway here is a 'fixed' version that should address some of the problems you guys raised:

/* passgen.c: Generate pseudo-random passwords 
   Copyright (C) David Futcher (bobbo) 2008
   <bobbocanfly@gmail.com> - rev 2

   Release under the GNU GPL License v2 or above */

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

int main(int argc, char** argv)
{
    int bytes, pass, n, i;
    char * buffer;

    /* Get user args */
    if (argc >= 3)
    {
        pass = atoi(argv[1]);
        bytes = atoi(argv[2]);

        /* Detects incorrect or stupid input */
        if (pass < 1) 
        {
            printf("%s <no. passwords to generate> <length of passwords>\n", argv[0]);
            return 1;
        }
        else if (bytes < 1)
        {
            printf("%s <no. passwords to generate> <length of passwords>\n", argv[0]);
            return 1;
        }
    }
    else
    {
        printf("%s <no. passwords to generate> <length of passwords>\n", argv[0]);
        return 1;
    }

    /* Set up dynamic memory + rand() seed */
    buffer = malloc(bytes+1);

    if (buffer == NULL)
    {
        printf("Could not allocate memory...exiting\n");
        return 1;
    }

    srand(time(NULL));

    /* Actual generation loop */
    for (i=0; i<pass; i++)
    {
        for (n=0; n<bytes; n++)
        {
            buffer[n] = 'a' + (31.0 * (rand() / (RAND_MAX + 1.0)));
        }
        
        buffer[bytes] = '\0';
    
        printf ("%s\n", buffer);
    }

    /* It worked! Get ready to exit */
    free(buffer);
    return 0;

}