Reading, writing, and copying files

Originally, I created a program that echoes files or strings to output. This is a modification of that same program and goes a step further. It does work (in both Linux and Windows), but I can NOT guarantee that it is bug free or that it is even fully compatible with both OS's.

This program writes, reads, and copies files. When writing to a file, it appends the line
buffered input to the file. This program works best with valid text based files. I can NOT guarantee any behavior since the program does not attempt to check the files actual file type; merely the extension name.

The main differences are:

• It solicits the user for input instead of operating from the CLI

• It operates only on (or with) files

• It requires the use of "keywords" to operate

I'm looking for helpful and useful critiques, mainly in programming style, organization, and if it's easily understood (or if there was difficulty in understanding) source code. If any critiques are made, please add in how I can improve upon my "misdeeds".

(Source files are linked via Pastebin)

nanproto.h

```
/ ****
set the preprocessor directives
/

/*
SET THE SWITCHES
----------------
To avoid conflicts while editing,
and recompiling, I used a
"switch mechanism" to turn them
ON and OFF.
*/
#ifndef ON
# define ON 1
#endif

#ifndef OFF
# define OFF 0
#endif

/*
Set the ERROR macro
*/
#ifndef ERROR
# define ERROR -1
#endif

/*
maximum string length
---------------------
11 //tiny
21 //small
41 //medium
81 //large
101 //extra
---------------------
all sizes are offset by one
to include the null character
*/
#ifndef SLEN
# define SLEN 81
#endif

/*
Set the boolean values for the
variables true and false...
*/
#ifndef BOOL
# define BOOL ON
# if BOOL
#

Warnings

Pay attention to compiler warnings! Just because they're called "warnings" doesn't mean you can ignore them. They're errors. Fix them.

The most abundant warning is when you're printing line numbers in nanfile.c.

printf("[%ld]: ", line);

The line variable is a long long, so the format string should use %lld.

In the same file, you have a number of int value variables that aren't used for anything. Get rid of them.

In nanstring.c, you assign the result of strlen to an int. This should be size_t.

Now for two pairs of errors. In nanstring.c, my compiler (clang) complains that option might not be initialized when it is returned. Meanwhile, in nanite.c, it complains that you're comparing option against ERROR, which can never be true because -1 is not a possible enum value. You're returning an int from menu, which is poor practice. Instead, add an error case to the select enum.

enum select {
    copy, help, line,
    quit, read, write,
    error = -1
};

Then, you can change the return types of menu and find_key to enum select. I also don't like this at all:

for (option = copy; option <= write; option++)
{
    if (0 == strcmp(source, keyletters[option]))
    {
        key_is_found = true;
        break;
    }
}

First of all, looping over a return value as a counter is confusing. Second of all, the flag variable key_is_found is unnecessary. Change the name of option to result, initialize it with error and change the loops to this:

for (enum select option = copy; option <= write; option++)
{
    if (0 == strcmp(source, keyletters[option]))
    {
        result = option;
        break;
    }
}

Then you can remove the flag boolean and the conditional at the end of the function, reducing it to a simple return result;, and the warning is gone! Killing a few birds with one stone.

The API and nanproto.h

This is pretty nice. I like how clearly structured and well-documented everything is. I just have a few minor notes.

#ifndef BOOL
#   define BOOL ON
#   if BOOL
#       define true 1
#       define false 0
#   endif
#endif

C99 includes the system header file stdbool.h, which has a typedef that maps _Bool to bool and includes true and false constants. You should probably just use that, considering you're using _Bool, anyway.

#ifndef BUFSIZE
#   define BUFSIZE 1024
#endif

I find this a little strange. It would make more sense to limit by lines, not by buffer size. This feels like an implementation detail the user should never see, but in fact it affects the program's behavior noticeably.

Also, you note this:


Prototypes were left optional but make a good reference and allows the "black-box" concept to stay in play.

Why are these optional? These should always be on.

nanstring.c

This code is very well written. Building nice CLI tools in C can be difficult, but this pulls it off with aplomb. Again, some minor notes.

From pause_buffer:

if (!isspace(ch))
    while (getchar() != '\n') continue;

This isn't terrible, but I'd recommend adding braces to, at the very least, the if statement, just for readability's sake. I'd give the same advice for the function just below it, string_to_lower:

for (int index = 0; string[index]; index++)
    if (isalpha(string[index]))
        string[index] = tolower(string[index]);

nanfile.c

Still quite solid code. The documentation comments continue to be excellent, and the code style is consistent, clean, and readable.

The biggest issue I found was in known_file_extension. Specifically, right here:

//find the last occurring period
char * file_extension_type  = strrchr(filename, '.');

What if there's no period in the filename? Segfault! Make sure you handle that case, too.

Another concern here is repetition. The read_file, read_line, write_file, and copy_file functions are all conspicuously similar. Extract the shared functionality into helper functions! Then the others should become quite trivial.

Here are the main components I identify as needing to be shared:

• validate_file_extension — Checks if a file extension is valid and prints a message on failure.

• print_numbered_line — Prints a line with a preceding line number.

• check_file_existence — Prints out messages if a file is being created or overwritten.

At the same time, I recognize that since you're using the I/O API fairly efficiently, it's hard to modularize the differences, especially in a language like C in which abstraction doesn't come easily or cheaply. It would be nice to make copy_file a composition between read_file and write_file, but I understand that wouldn't be as clean.

In both read_file and read_line, consider padding the line numbers so that transitions from single-digit to double-digit to triple-digit line numbers don't push the whole output to the right by a single character.

Now, here's another important point. This code

StackExchange Code Review Q#78448, answer score: 4