Skip to main content
added 32 characters in body
Source Link
SKi
  • 450
  • 1
  • 4
  • 10

Global variables:

  • If those global variables are not changed, make those constant.
  • If those are used only in the C file, you can make those static.
  • Don't set magic number 52 to alphabetSize, use sizeof when possible.
  • Don't mix camel-case (alphabetSize) and underscores (max_length).

Instead of this:

char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int alphabetSize = 52;
int max_length = 4;

You can do it this way:

static const char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const int ALPHABET_SIZE = sizeof(ALPHABET) - 1;
static const int MAX_LENGTH = 4;

Function parameters:

  • If function is not going to change data behind pointer parameter, use const keyword with those parameters.
  • Instead of string, I propose to use char *.

Instead of this:

char *crack(string hash, string salt) 

Use this:

static char *crack(const char *hash, const char *salt)

Efficiency:

I assume that most of the time is spend in the crypt() call. If not, then there is one place for (useless) micro-optimization: Change the following line:

sprintf(buf + index, "%c", ALPHABET[i]);

to:

buf[index] = ALPHABET[i];
buf[index+1] = '\0';

Global variables:

  • If those global variables are not changed, make those constant.
  • If those are used only in the C file, you can make those static.
  • Don't set magic number 52 to alphabetSize, use sizeof when possible.
  • Don't mix camel-case and underscores.

Instead of this:

char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int alphabetSize = 52;
int max_length = 4;

You can do it this way:

static const char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const int ALPHABET_SIZE = sizeof(ALPHABET) - 1;
static const int MAX_LENGTH = 4;

Function parameters:

  • If function is not going to change data behind pointer parameter, use const keyword with those parameters.
  • Instead of string, I propose to use char *.

Instead of this:

char *crack(string hash, string salt) 

Use this:

static char *crack(const char *hash, const char *salt)

Efficiency:

I assume that most of the time is spend in the crypt() call. If not, then there is one place for (useless) micro-optimization: Change the following line:

sprintf(buf + index, "%c", ALPHABET[i]);

to:

buf[index] = ALPHABET[i];
buf[index+1] = '\0';

Global variables:

  • If those global variables are not changed, make those constant.
  • If those are used only in the C file, you can make those static.
  • Don't set magic number 52 to alphabetSize, use sizeof when possible.
  • Don't mix camel-case (alphabetSize) and underscores (max_length).

Instead of this:

char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int alphabetSize = 52;
int max_length = 4;

You can do it this way:

static const char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const int ALPHABET_SIZE = sizeof(ALPHABET) - 1;
static const int MAX_LENGTH = 4;

Function parameters:

  • If function is not going to change data behind pointer parameter, use const keyword with those parameters.
  • Instead of string, I propose to use char *.

Instead of this:

char *crack(string hash, string salt) 

Use this:

static char *crack(const char *hash, const char *salt)

Efficiency:

I assume that most of the time is spend in the crypt() call. If not, then there is one place for (useless) micro-optimization: Change the following line:

sprintf(buf + index, "%c", ALPHABET[i]);

to:

buf[index] = ALPHABET[i];
buf[index+1] = '\0';
Source Link
SKi
  • 450
  • 1
  • 4
  • 10

Global variables:

  • If those global variables are not changed, make those constant.
  • If those are used only in the C file, you can make those static.
  • Don't set magic number 52 to alphabetSize, use sizeof when possible.
  • Don't mix camel-case and underscores.

Instead of this:

char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int alphabetSize = 52;
int max_length = 4;

You can do it this way:

static const char ALPHABET[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const int ALPHABET_SIZE = sizeof(ALPHABET) - 1;
static const int MAX_LENGTH = 4;

Function parameters:

  • If function is not going to change data behind pointer parameter, use const keyword with those parameters.
  • Instead of string, I propose to use char *.

Instead of this:

char *crack(string hash, string salt) 

Use this:

static char *crack(const char *hash, const char *salt)

Efficiency:

I assume that most of the time is spend in the crypt() call. If not, then there is one place for (useless) micro-optimization: Change the following line:

sprintf(buf + index, "%c", ALPHABET[i]);

to:

buf[index] = ALPHABET[i];
buf[index+1] = '\0';