Return to Revisions

Nicely laid-out; easy to read and follow.

Avoid unnecessary complexity

Code is short & simple enough that (IMO) it should be combined into a single source file, eliminating "app header file" complexities (including procedural code/functions defined in the header file.) This extends to ancillary 'makefile' maintenance, too.

The separation of main() and the two functions suggests an intent to prepare for variations of this program. "Premature optimisation" should be avoided.

Optimising, now, for a (presumed) vague concept of future versions or enhancements has cost time & effort that may never be needed. Further, it's likely that whatever main() is doing to parse command line parameters will need to be changed, as will (at least) the function signatures of the two functions (or their future counterparts.) It's good to plan ahead, but it's better to let the code and your plans illuminate your footsteps. Let the code 'tell you' when it should be separated into multiple source files (with headers).

Be consistent

main() contains (abridged):

 int opt = -1;
 while((opt = getopt(argc, argv, "dk:")) != -1) {
 ...
 }

yet flipcrypt() stretches things out with:

 int tmpchar;
 tmpchar = fgetc(fpin);
 while (tmpchar != EOF) {
 ...
 tmpchar = fgetc(fpin);
 }

Both idioms work. Switching from one to the other in the same program can be distracting.

Note: The scope of the local variables could be made tighter and more obvious with:

 for( T rval = constant; ( rval = operation( parms ) ) != constant; /**/ ) {
 // use rval
 }
 // 'rval' is no longer in scope...

When it comes to writing/reading code, often less is more.

Token breeding

#define FC_ENCRYPT 1
#define FC_DECRYPT 0
#define RB_LEFT 1
#define RB_RIGHT 0

Thoughtful examination of the processing reveals the RB_* tokens are not really necessary. Their "information value" might be better expressed in simple comments proximate to the shifting operation statements for EN-/DE-cryption.

Suggestion: enum { e_DECR, e_ENCR }; as a one line alternative basis.

• enum tokens are available to debuggers whereas preprocessor tokens are lost.
• Note that shorter token names are easier to distinguish.
• Less is more.

C strings end with NUL

Abridged code:

 int keylen = strlen(key);
 int ik = 0;
 ...
 ik++;
 if(ik == (keylen)) ik = 0;

could be reduced by testing for the end of the key string:

 int ik = 0;
 ...
 ik++;
 if( key[ ik-1 ] == '0円' || key[ ik ] == '0円' ) ik = 0;

Note: Eliminating strlen() (and the header #include) reveals a problem if the user provides a zero-length string as the key. The original code, testing for equality after incrementing, exhibits undefined behaviour by accessing well beyond that single-byte string buffer. Not good...

^{% progname -k "" < plain.txt > cipher.bin uses a zero length key string.}

Subtleties are hazards

The fputc() is subtly truncating hi-order bits of the int value after the shifting operations. Lacking explicit truncation to retain only low-order bits will be revealed as 'corrupted data' when higher level functionality is changed to assemble packets of 'bytes' to suit some future purpose.

Suggested:

static int rotatebits(int inchar, int offset, int lorr) {
 ...
 return tmpchar & 0xFF; // See below
}

Left as an exercise: Use CHAR_BIT to have the compiler generate the appropriate mask constant (instead of 0xFF).

Cryptography tidbit

Pages of a "one time pad" contain MANY random values used to encipher each plain text character. Security is enhanced by the "key" being (ideally) as long as the message (so that the enciphering operation does not use a "repeat" of the key's sequence.)

Consider your algorithm with a one-character 'key'. Each enciphered letter is processed exactly the same (easy to break). Consider using a five-character 'key' for a 1000 character message. The key has been 're-used' 200 times (providing lots of repetition for the code-breaker to use to crack the code).

Left as an exercise: Improve the algorithm to increase the robustness of the cipher.

Trivia: The famous Enigma cipher machine is legendary for its squillions of enciphering possibilities. The wiring between its plugboard and "input rotor" could have been 'scrambled', thereby increasing the number of possibilities to be 1 out of 26! (a VERY large value). Instead, that bit of wiring was established in "ascending alphabetic order". Of the 4✕1026 wiring possibilities, the Germans used THE MOST OBVIOUS ONE... (A major 'oops!')