I’m a C beginner. I was learning about getopt to parse command line options and I decided to write this small XOR cipher program for files.

It acts as a “UNIX filter” (I guess) since default input and output are stdin and stdout. It can generate a random key file from /dev/urandom if the provided one doesn’t exists (size of this file can also be specified).

I split the thing in three files and here they are:

fxor.h

#ifndef FXOR_HEADER #define FXOR_HEADER  #include <stdio.h> #include <stdbool.h>  #define URANDOM "/dev/urandom"  /*  * fxor:  applies XOR cipher to bytes from in file using bytes from key file as  *        key, writes result to out file and returns true.  *        If key file is smaller than in file, key file will restart from  *        beginning each time EOF is reached.  *  *        in and key files are assumed to be opened in read mode, out file in  *        write mode. in, out, key can't be the same file, otherwise behaviour  *        is undefined.  *  *        If an error occurs while writing to out, returns false.  */ bool fxor(FILE *in, FILE *out, FILE *key);  /*  * fwrite_urandom:  writes nbytes bytes to out file from URANDOM and returns  *                  true. out file is assumed to be opened in write mode.  *                  If URANDOM can't be opened or an error occurs while writing  *                  to out, returns false.  */ bool fwrite_urandom(FILE *out, size_t nbytes);  #endif 

fxor.c

#include "fxor.h"   bool fxor(FILE *in, FILE *out, FILE *key) {         unsigned char in_byte;         unsigned char key_byte;         int c;         while ((c = fgetc(in)) != EOF) {                 in_byte = c;                  if ((c = fgetc(key)) == EOF) {                         rewind(key);                         c = fgetc(key);                 }                 key_byte = c;                  if (fputc(in_byte ^ key_byte, out) == EOF) {                         return false;                 }         }         return true; }  bool fwrite_urandom(FILE *out, size_t nbytes) {         FILE *urandom;         if ((urandom = fopen(URANDOM, "rb")) == NULL) {                 return false;         }         while (nbytes-- > 0) {                 if (fputc(fgetc(urandom), out) == EOF) {                         fclose(urandom);                         return false;                 }         }         fclose(urandom);         return true; } 

main.c

#include <stdio.h> #include <errno.h> #include <getopt.h> #include <ctype.h> #include <stdint.h>  #include "fxor.h"  #define DEFAULT_KEY_LENGTH 65536  /*  * usage:  print an usage message.  */ static void usage(const char *prog_name);  /*  * atost:  convert str to a size_t value and returns it.  *         No check for overflow is made, if str contains an invalid size_t  *         value, SIZE_MAX will be returned. str can start with '+'. If str is  *         an empty string 0 will be returned.  */ static size_t atost(const char *str);  /*  * open_or_gen_key:  if key_filename exists, it will be opened in read mode and  *                   the pointer returned, otherwise, if it doesn't exists, it  *                   will be created, its size will be key_length bytes and it  *                   will be filled with bytes from URANDOM. A pointer to it  *                   will be returned after reopening the file in read mode.  *                   If an error occurs, returns NULL.  */ static FILE *open_or_gen_key(const char *key_filename, size_t key_length);  /*  * fclose_all:  close three file it gets as input. It is safe to pass NULL  *              pointer as file, no operation on it will be performed.  */ static void fclose_all(FILE *in, FILE *out, FILE *key);   static void usage(const char *prog_name) {         printf("usage: %s -k key [-l key_length] [-i input] [-o output] [-h]\n\n"                "  -k key         key filename, required. If doesn't exists, a file\n"                "                 filled with bytes from %s will be created and\n"                "                 used for xor operation, default size will be %d bytes,\n"                "                 you can specify a different one with -l option.\n\n"                "  -l key_length  optional size in bytes for key file if has to be generated,\n"                "                 must be >= 0. If -k file exists, -l will not be used.\n\n"                "  -i input       input file, default standard input.\n\n"                "  -o output      output file, default standard output.\n\n"                "  -h             show this message and exit.\n",                prog_name, URANDOM, DEFAULT_KEY_LENGTH); }  static size_t atost(const char *str) {         size_t value = 0;         for (size_t i = (str[0] == '+') ? 1 : 0;              str[i] != '';              i++) {                 if ( ! isdigit(str[i])) {                         return SIZE_MAX;                 }                 value = value * 10 + (str[i] - '0');         }         return value; }  static FILE *open_or_gen_key(const char *key_filename, size_t key_length) {         FILE *key;         if ((key = fopen(key_filename, "rb")) == NULL && errno == ENOENT) {                 if ((key = fopen(key_filename, "wb")) == NULL) {                         return NULL;                 }                 if ( ! fwrite_urandom(key, key_length)) {                         fclose(key);                         return NULL;                 }                 key = freopen(key_filename, "rb", key);         }         return key; }  static void fclose_all(FILE *in, FILE *out, FILE *key) {         if (in != NULL) {                 fclose(in);         }         if (out != NULL) {                 fclose(out);         }         if (key != NULL) {                 fclose(key);         } }  int main(int argc, char **argv) {         char *in_filename, *out_filename, *key_filename, *key_length_str;         in_filename = out_filename = key_filename = key_length_str = NULL;          int o;         while ((o = getopt(argc, argv, "k:l:i:o:h")) != -1) {                 switch (o) {                 case 'k':                         key_filename = optarg;                         break;                 case 'l':                         key_length_str = optarg;                         break;                 case 'i':                         in_filename = optarg;                         break;                 case 'o':                         out_filename = optarg;                         break;                 case 'h':                         usage(argv[0]);                         return 0;                 default:                         return 1;                 }         }         if (key_filename == NULL) {                 fprintf(stderr, "%s: option is required -- 'k'\n", argv[0]);                 return 1;         }          FILE *in = stdin;         FILE *out = stdout;         FILE *key = NULL;         size_t key_length = DEFAULT_KEY_LENGTH;          if (key_length_str != NULL && (key_length = atost(key_length_str)) == SIZE_MAX) {                 fprintf(stderr, "%s: invalid argument for option -- 'l'\n", argv[0]);                 return 1;         }         if (in_filename != NULL && (in = fopen(in_filename, "rb")) == NULL) {                 perror(in_filename);                 return 1;         }         if (out_filename != NULL && (out = fopen(out_filename, "wb")) == NULL) {                 perror(out_filename);                 fclose_all(in, out, key);                 return 1;         }         if ((key = open_or_gen_key(key_filename, key_length)) == NULL) {                 perror(key_filename);                 fclose_all(in, out, key);                 return 1;         }          if ( ! fxor(in, out, key)) {                 fprintf(stderr, "%s: error while applying xor\n", argv[0]);                 fclose_all(in, out, key);                 return 1;         }          fclose_all(in, out, key); } 

Example:

$   cc -std=c11 -Wall -Wextra -Werror -o fxor fxor.c main.c  $   l fxor*  fxor.c  fxor.h  main.c $   echo "Hello fxor!" | ./fxor -k hello.key -l 16 -o hello.xor $   ll | grep "hello*" -rw-r--r-- 1 marco marco    16 Nov 18 16:16 hello.key -rw-r--r-- 1 marco marco    12 Nov 18 16:16 hello.xor $   ./fxor -i hello.xor -k hello.key  Hello fxor! $   xxd -b hello.key  00000000: 10000010 01110111 01110010 01110001 01110100 10011111  .wrqt. 00000006: 00111111 10110001 10110001 10011011 10111010 00111011  ?....; 0000000c: 01110010 01001000 11100110 11001111                    rH.. $   xxd -b hello.xor 00000000: 11001010 00010010 00011110 00011101 00011011 10111111  ...... 00000006: 01011001 11001001 11011110 11101001 10011011 00110001  Y....1 $   ./fxor -i hello.xor -k hello.key | xxd -b 00000000: 01001000 01100101 01101100 01101100 01101111 00100000  Hello  00000006: 01100110 01111000 01101111 01110010 00100001 00001010  fxor!. $   

I’d like to get some feedback about style and advices about dealing with files and errors: how can I improve the “if cascade” made of “if fopen fails, print an error, close things opened before and return 1“?

Also I bet that opening a device just to get random bytes isn’t the best thing to do. Do you have any suggestion on that?