C programming beginner | self-study CS course

pxavier · 2012-08-14 18:19:48

Hello,
I am taking a self-study computer science course (cs50 OpenCourseWare - http://cs50.tv/2011/fall/).
I have just completed the problem set two and I would like to know if it could be possible to receive some feedback on my code.

Here is my source code: http://ideone.com/L04ep
Here is the problem set specification: http://cdn.cs50.net/2011/fall/psets/2/hacker2.pdf (from page 6<10 to page 7<10 )

Any critics would be greatly appreciated.

sujoy · 2012-08-15 05:15:35

@pxavier

just my opinion,

instead of "#define CORRECT_ARG_COUNT 2" , I'd rather use, "const int " here to ensure the type checking.
similarly for the rest.

Trent · 2012-08-16 14:27:51

sujoy wrote:

instead of "#define CORRECT_ARG_COUNT 2" , I'd rather use, "const int " here to ensure the type checking.
similarly for the rest.

Fine for CORRECT_ARG_COUNT, but BUFFER_SIZE, SALT_LEN, MAX_PASSWD_LEN, and NBR_OF_PRINTBL_ASCII_CHARS must be constant expressions because they're used between the [] of array definitions. A const int isn't a constant expression in C89. C99 will make them VLAs instead I think, so if that's acceptable it might not make any difference. (I hate VLAs and try never to use them.)

pxavier · 2012-08-16 15:07:15

sujoy wrote:

instead of "#define CORRECT_ARG_COUNT 2" , I'd rather use, "const int " here to ensure the type checking.

What do you mean by "ensure the type checking" ?

Trent wrote:

I hate VLAs and try never to use them

Why ?

By the way, line 236, I have realized that at some point an int will be too small to hold the value of count.
It should be a long long int instead of an int... (stupid mistake)

fixed: http://ideone.com/c1Pc1

Trent · 2012-08-17 14:06:23

pxavier wrote:

Trent wrote:
I hate VLAs and try never to use them
Why ?

VLAs were tacked on to the C99 standard at the request of no one who ever programmed in C. They're messy to implement safely and increase the complexity of the compiler unnecessarily. They can only be automatic. They are allocated on the stack at runtime, which makes it impossible to predict how quickly your stack will grow (and easier to accidentally overflow it by passing a large number to an otherwise inocuous function). They silently defer evaluation of `sizeof` to runtime. In short, VLAs do not belong in a small and elegant language like C, and are at best a poor replacement for malloc() / free(). Even the C standard committee realized its mistake and made VLAs optional in C11 (there's a macro you can test, __STDC_NO_VLA__, to determine whether they're supported by your implementation).

By the way, line 236, I have realized that at some point an int will be too small to hold the value of count.
It should be a long long int instead of an int... (stupid mistake)

Maybe consider using an unsigned type for values that are counts? It's good documentation. Although be careful when you use them for math; integer promotions can sometimes have surprising results.

BTW, I noticed something else while looking at that section:

                for (long long int count = 0; count < pow(BASE, passwd_len); count++) {                   
                        ...
                }

(a) It's usually not advisable to use pow when raising an integer to an integer power and requiring an integer result. For small numbers it may well work fine, but when the number of bits of the result exceeds that of the significand of a double (and possibly a little before that), you'll be prone to rounding errors. Since floating point and integer implementations vary, you'd be well advised to just do it with integer arithmetic. Look into exponentiation by squaring for a reasonably quick implementation of an integer power function.

(b) passwd_len never changes while this loop runs, but you still have to compare against count (and passwd_len is not a constant expression that could be easily optimized), so the pow() function gets called every time through the loop when you only need it evaluated once. Call it beforehand and store the result in a variable.

Nisstyre56 · 2012-08-19 00:08:11

pxavier wrote:

What do you mean by "ensure the type checking" ?

They mean that if you make it a macro, then it will not be type checked (it would only be type checked if you used the macro in a variable declaration or in some other C construct that requires type checking), so if it's a macro, then it can conceivably be any type, and thus not type checked.

e.g.

#include <stdio.h>
#define FOO 97

int main(void) {
    int a = FOO;
    double b = FOO;
    char c = FOO;
    printf("%c %d %f", c, a, b);
    return 0;
}

The macro FOO here is not type checked, and thus can take on the form of any possible type.

EDIT: I just realized that your question could have been more "what is type checking?", so I'll do my best to explain that.

In statically typed languages, such as C, types of everything are checked at compile time, meaning after the program has been parsed, and before it's been run or had any code generated in the target language (target language being assembly for C). So, there are various ways of doing this, and the easiest one to implement is making the programmer give "hints" to the compiler about what the types of things are, and that is why you have to specify the parameter and return types of procedures, the types of variables, and so on. When you use a macro, the only thing it does is modify your program's source code by taking all of the places where "FOO" is and replacing it with the string on the right hand side of the define macro. This is BEFORE the type checking phase.

Does that make sense?

Last edited by Nisstyre56 (2012-08-19 01:12:52)

ngnmhieu · 2013-07-29 10:16:16

pxavier wrote:

Hello,
I am taking a self-study computer science course (cs50 OpenCourseWare - http://cs50.tv/2011/fall/).
I have just completed the problem set two and I would like to know if it could be possible to receive some feedback on my code.
Here is my source code: http://ideone.com/L04ep
Here is the problem set specification: http://cdn.cs50.net/2011/fall/psets/2/hacker2.pdf (from page 6<10 to page 7<10 )
Any critics would be greatly appreciated.

Hi, i'm really interested in your algorithm that generate all posible permutation of password.
I don't really understand this block of code

for (int passwd_len = 1; passwd_len <= MAX_PASSWD_LEN; passwd_len++) {
                for (int count = 0; count < pow(BASE, passwd_len); count++) {                   
                        for (int index = 0, power = passwd_len - 1; index < passwd_len; index++, power--) {
                                potential_passwd[index] = printbl_ascii_chars[(int) (count / (pow(BASE, power))) % BASE];
                        }
                        
                        potential_passwd[passwd_len] = '\0';
                        
                        if (!try_potential_passwd(potential_passwd, ciphertext)) {
                                cracked = true;
                                printf("%s\n", potential_passwd);
                                break;
                        }
                }
                
                if (cracked)
                        break;
        }

especially this line

potential_passwd[index] = printbl_ascii_chars[(int) (count / (pow(BASE, power))) % BASE];

can clarify that? thanks!

Barrucadu · 2013-07-30 22:32:58

edit: Sorry to post in an old thread, I didn't notice the original post times, only that it had been bumped.

Nisstyre56 wrote:

They mean that if you make it a macro, then it will not be type checked (it would only be type checked if you used the macro in a variable declaration or in some other C construct that requires type checking), so if it's a macro, then it can conceivably be any type, and thus not type checked.
e.g.
#include <stdio.h>
#define FOO 97

int main(void) {
    int a = FOO;
    double b = FOO;
    char c = FOO;
    printf("%c %d %f", c, a, b);
    return 0;
}
The macro FOO here is not type checked, and thus can take on the form of any possible type.

It's not that FOO isn't type-checked, what happens is that every instance of FOO is replaced with 97. This happens before type-checking. Type-checking then happens on the resultant code.

Last edited by Barrucadu (2013-07-30 22:33:56)

Arch Linux

#1 2012-08-14 18:19:48

C programming beginner | self-study CS course

#2 2012-08-15 05:15:35

Re: C programming beginner | self-study CS course

#3 2012-08-16 14:27:51

Re: C programming beginner | self-study CS course

#4 2012-08-16 15:07:15

Re: C programming beginner | self-study CS course

#5 2012-08-17 14:06:23

Re: C programming beginner | self-study CS course

#6 2012-08-19 00:08:11

Re: C programming beginner | self-study CS course

#7 2013-07-29 10:16:16

Re: C programming beginner | self-study CS course

#8 2013-07-30 22:32:58

Re: C programming beginner | self-study CS course

Board footer