Structure and style of Enigma Machine

I took a little time and wrote the following code to produce enigma encryption. I don't normally write code in C so I would like to get feedback on the way it has been structured and any issues a more experienced C programmer might spot in the way I used structs, command line parsing, and the indexing around the strings to replicate the turning of rotors.

I did not create a separate header file since the code is so short I thought it would be best to just have a single file.

```
#include
#include
#include
#include

#define ROTATE 26

const char *alpha = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
const char *rotor_ciphers[] = {
"EKMFLGDQVZNTOWYHXUSPAIBRCJ",
"AJDKSIRUXBLHWTMCQGZNPYFVOE",
"BDFHJLCPRTXVZNYEIWGAKMUSQO",
"ESOVPZJAYQUIRHXLNFTGKDCMWB",
"VZBRGITYUPSDNHLXAWMJQOFECK",
"JPGVOUMFYQBENHZRDKASXLICTW",
"NZJHGRCXMYSWBOUFAIVLPEKQDT",
"FKQHTLXOCBJSPDZRAMEWNIUYGV"
};

const char *rotor_notches[] = {"Q", "E", "V", "J", "Z", "ZM", "ZM", "ZM"};

const char *rotor_turnovers[] = {"R", "F", "W", "K", "A", "AN", "AN", "AN"};

const char *reflectors[] = {
"EJMZALYXVBWFCRQUONTSPIKHGD",
"YRUHQSLDPXNGOKMIEBFZCWVJAT",
"FVPJIAOYEDRZXWGCTKUQSBNMHL"
};

struct Rotor {
int offset;
int turnnext;
const char* cipher;
const char* turnover;
const char* notch;
};

struct Enigma {
int numrotors;
const char* reflector;
struct Rotor rotors[8];
};

/*
* Produce a rotor object
* Setup the correct offset, cipher set and turn overs.
*/
struct Rotor new_rotor(struct Enigma *machine, int rotornumber, int offset) {
struct Rotor r;
r.offset = offset;
r.turnnext = 0;
r.cipher = rotor_ciphers[rotornumber - 1];
r.turnover = rotor_turnovers[rotornumber - 1];
r.notch = rotor_notches[rotornumber - 1];
machine->numrotors++;

return r;
}

/*
* Return the index position of a character inside a string
* if not found then -1
**/
int str_index(const ch

During the command line parsing I would suggest using else

This

if (strcmp(argv[i], "-d") == 0) opt_debug = 1;
    if (strcmp(argv[i], "-r") == 0) {

Would be better written as

if (strcmp(argv[i], "-d") == 0) opt_debug = 1;
    else if (strcmp(argv[i], "-r") == 0) {
           ....

To avoid unnecessary checks. (If it is -d then it cannot also be -r so why check?).

I would suggest you remove the i++ in the middle of the loop. This is a problem because if someone only enters -r as a single command line argument then this line will access invalid memory. if (strcmp(argv[i], "-o")...

/* argc = 2, argv[1] = "-r" */
for (i = 1; i < argc; i++){
    if (strcmp(argv[i], "-d") == 0) opt_debug = 1;
    if (strcmp(argv[i], "-r") == 0) {
        opt_r1 = atoi(&argv[i+1][0])/100;
        opt_r2 = atoi(&argv[i+1][1])/10;
        opt_r3 = atoi(&argv[i+1][2]);
        i++; /* i = 2 */
    }
    /* There is no argv[2]! */
    if (strcmp(argv[i], "-o") == 0) {

It makes your code simpler if you leave the loop mechanics to the loop declaration for (...) and not mess around with the loop counter inside the loop.

If you're expecting an argument after -r you should explicitly check that it exists.

/* Is i+1 < argc at this point? */
        opt_r1 = atoi(&argv[i+1][0])/100;
        opt_r2 = atoi(&argv[i+1][1])/10;
        opt_r3 = atoi(&argv[i+1][2]);

And the same goes for the length of these arguments. If argv[i+1] exists but is only 1 character long then you will be accessing invalid memory on the third atoi line. Use strlen to make sure the argument is as long as you expect it or check for the end of string '\0' character before each atoi.

On a style note, as someone reading your code I would ask that you not alternate between char c, char c and char *c. Like everything related to code style there is no "right" way or "wrong" way but it will help your readers if you pick a style and stick to it.

StackExchange Code Review Q#49288, answer score: 3