7
\$\begingroup\$

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:

  1. It solicits the user for input instead of operating from the CLI
  2. It operates only on (or with) files
  3. 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
#               define true 1
#               define false 0
#       endif
#endif

/*
        Define the menu options.
        -----------------------------------
        These options help delegate the
        menu's I/O by using the find_key()
        known_file_extension(), and menu()
        functions defined in the nanite.c,
        nanstring.c, and nanfile.c files.
        -----------------------------------
        More keywords can be added by simply
        changing the KEYLEN macro value.
        The strings can be found in the
        nanstring.c file.
*/
#ifndef MENU_OPTIONS
#       define MENU_OPTIONS ON
#       if MENU_OPTIONS == ON
#               define KEYLEN 6
                enum select {
                        copy, help, line,
                        quit, read, write
                };

                const char * keywords[KEYLEN];
                const char * keyletters[KEYLEN];
#       endif
#endif

/*
        maximum buffer size
        -------------------
        const long buffer_size = 512; //tiny buffer
        const long buffer_size = 1024; //small buffer
        const long buffer_size = 2048; //medium buffer
        const long buffer_size = 4096; //large buffer
        const long buffer_size = 8192; //extra large buffer
*/
#ifndef BUFSIZE
#       define BUFSIZE 1024
#endif

/*
        Define the file options.
        ------------------------
        similar to menu options, these settings
        are used to decide whether the given
        file extension type is a "valid" one.
        considering there are more effecient
        methods and this one is a trite and tried
        method, its used for educational purposes.
        -----------------------------------
        More extensions can be added by
        simply changing the EXTLEN macro
        value. The strings can be found
        in the nanstring.c file.
        Keep in mind that the order of the
        values and string elements must
        be the same.
*/
#ifndef FILE_OPTIONS
#       define FILE_OPTIONS ON
#       if FILE_OPTIONS == ON
#               define EXTLEN 9
                enum file { txt, asc, c, h, csv, html, log, xhtml, xml };

                const char * extension[EXTLEN];
#       endif
#endif

/*
        Prototypes were left optional
        but make a good reference and
        allows the "black-box" concept
        to stay in play.
        Once a function has been tested,
        and works, it can be added to the
        prototype list.
*/
#ifndef PROTOTYPES
#       define PROTOTYPES ON
#       if PROTOTYPES == ON
                /* *********************************
                MENU BASED FUNCTIONS
                --------------------
                Prototypes for the nanstring.c file
                --------------------
                these functions operate on strings
                ************************************/
                void eatline(void);
                void remove_newline(char *);
                void pause_buffer(void);
                void string_to_lower(char *);
                int find_key(char *);
                int menu(char *);
                void display_help(void);
                void prompt(void);

                /* *********************************
                FILE BASED FUNCTIONS
                --------------------
                Prototypes for the nanfile.c file
                --------------------
                these functions operate on files
                ************************************/
                int known_file_extension(const char *);
                int read_file(const char *);
                int read_line(const char *, long long);
                int write_file(const char *);
                int copy_file(const char *, const char *);
#       endif
#endif

nanite.c

/*
*************************************************************
Written by: JargonJunkie
*************************************************************
        This program reads, writes, and copies files.
*************************************************************
    This program was inspired by Problem 13-07 found in the 
    C Primer Plus book; Chp 13 Programming Exercises.
*************************************************************
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include "./nanproto.h"

int main(void)
{
    long long number;
    char input[SLEN], source[SLEN], target[SLEN];
    enum select option;

    prompt();

    while (quit != (option = menu(input)))
    {
        if (ERROR == option)
        {
            fprintf(stderr, "Invalid Command: %s\n", input);
            fputs("Starting over...\n\n", stderr);
            continue;
        }

        switch (option)
        {
            case copy:
                printf("(name source file)?> ");
                if (NULL == fgets(source, SLEN, stdin)) {
                    fprintf(stderr, "Could not read from standard input.\n\n");
                    break;
                }
                remove_newline(source);

                printf("(name target file)?> ");
                if (NULL == fgets(target, SLEN, stdin)) {
                    fprintf(stderr, "Could not read from standard input.\n\n");
                    break;
                }  
                remove_newline(target);
                putchar('\n');

                if (ERROR == copy_file(source, target))
                    fprintf(stderr, "[Error!]: Failed to copy %s to the file %s\n", source, target);
                break;

            case help:
                display_help();
                break;

            case line:
                printf("(name target file)?> ");
                if (NULL == fgets(source, SLEN, stdin)) {
                    fprintf(stderr, "Could not read from standard input.\n\n"); 
                    break;
                }
                remove_newline(source);

                printf("(enter line number)?> ");
                while (true != scanf("%lld", &number) || (number < 1))
                {
                    putchar('\n');
                    puts("Oops! Only Positive Integers Please...");
                    printf("(enter line number)?> ");
                    eatline();
                }
                eatline();
                putchar('\n');

                if (ERROR == read_line(source, number))
                    fprintf(stderr, "[Error!]: Failed to open the file %s\n\n", source);
                break;

            case read:
                printf("(name target file)?> ");
                if (NULL == fgets(source, SLEN, stdin)) {
                    fprintf(stderr, "Could not read from standard input.\n\n");
                    break;
                }
                remove_newline(source);
                putchar('\n');

                if (ERROR == read_file(source))
                    fprintf(stderr, "[Error!]: Failed to read the file %s\n\n", source);
                break;

            case write:
                printf("(name target file)?> ");
                if (NULL == fgets(source, SLEN, stdin)) {
                    fprintf(stderr, "Could not read from standard input.\n\n");
                    break;
                }
                remove_newline(source);
                putchar('\n');

                if (ERROR == write_file(source))
                    fprintf(stderr, "[Error!]: Failed to write to the file %s\n", source);
                break;

            default:
                fputs("Oops! Something went horribly wrong!\n", stderr);
                fprintf(stderr, "[Error] in main() -> while menu() -> switch (option)");
                exit(EXIT_FAILURE);
        }
    }

    putchar('\n');

    if (option == quit)
    {
        puts("Exit Success!");
        return 0;
    }
    else
    {
        puts("Exit Failure!");
        return 1;
    }
}

nanstring.c

/* *********************************
    NANITE STRINGs
    ---------------------------
    This file must be linked with the
    nanite.c and nanfile.c files.
    ---------------------------
    This file defines most of the executable
    code predefined by the "nanproto.h" file.
    These functions are used by the "nanfunct.c"
    and "nanite.c" source files.
************************************/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include "./nanproto.h"

/*
    Initialize KEYWORDS, KEYLETTERS, and EXTENSION
    ----------------------------------------------
    These keywords are used to compare against
    input supplied by the user. If the keyword
    is found to be valid, the function returns
    a value accordingly; else the function
    returns some type of error based value.
*/
const char * keywords[KEYLEN] = {
    "copy", "help", "line",
    "quit", "read", "write"
};

const char * keyletters[KEYLEN] = {
    "c", "h", "l",
    "q", "r", "w"
};

const char * extension[EXTLEN] = {
    ".txt", ".asc", ".c", ".h",
    ".csv", ".html", ".log",
    ".xhtml", ".xml"
};

/* *********************************
    MENU BASED FUNCTIONS
    --------------------
    these functions help create the
    operability of the menu() interface
    used in the nanite source file.
    these functions operate on strings
************************************/

/*
    The eatline() Function
    ---------------------------------------
    Dispose of input up to the newline
    character...
*/
void eatline(void)
{
    while (getchar() != '\n') 
        continue;
}

/*
    The remove_newline() Function
    ---------------------------------------
    Removes the last occurance of a newline
    character.
    If no newline is found, nothing is done.
    If a newline is found, it is replaced by
    the null character.
*/
void remove_newline(char * string)
{
    char * newline;
    size_t position, cpy_length, str_length = strlen(string);

    if ( NULL != (newline = strrchr(string, '\n')) )
    {
        cpy_length = strlen(newline);
        position = str_length - cpy_length;
        string[position] = '\0';
    }
}

/*
    The pause_buffer Function
    ---------------------------------------
    Causes the output buffer to stop until
    the user enters a newline.
*/
void pause_buffer(void)
{
    int ch;
    putchar('\n');
    puts("[enter] to continue...");
    ch = getchar();
    if (!isspace(ch))
        while (getchar() != '\n') continue;
}

/*
    The string_to_lower() function
    ---------------------------------------
    Transforms the string to lower case
*/
void string_to_lower(char * string)
{
    for (int index = 0; string[index]; index++)
        if (isalpha(string[index]))
            string[index] = tolower(string[index]);
}

/*
    The find_key() Function
    ---------------------------------------
    Compare the given keyword against the
    const keywords. IF the keyword is valid,
    return the elements location. ELSE return
    the ERROR macro.
*/
int find_key(char * source)
{
    int length, letters = 3;
    enum select option;
    _Bool key_is_found = false;

    string_to_lower(source);

    length = strlen(source);

    //compare the key letters
    if (length <= letters)
    {
        for (option = copy; option <= write; option++)
        {
            if (0 == strcmp(source, keyletters[option]))
            {
                key_is_found = true;
                break;
            }
        }
    }

    //compare the key words
    if (length > letters)
    {
        for (option = copy; option <= write; option++)
        {
            if (0 == strcmp(source, keywords[option]))
            {
                key_is_found = true;
                break;
            }
        }
    }

    //return the keywords position
    if (key_is_found) return option;
    //else keyword was not found
    else return ERROR;
}

/*
    The menu() Function
    ---------------------------------------
    Takes a string from input and returns
    a value to main() while loop. When
    a valid quit value is given, the loop
    is broken.
*/
int menu(char * string)
{
    int option;

    printf("(command)?> ");
    if (NULL == fgets(string, SLEN, stdin))
    {
        fprintf(stderr, "[menu()] -> Failed to successfully store string!");
        exit(EXIT_FAILURE);
    }
    else
        remove_newline(string);

    option = find_key(string);

    return option;
}

/*
    The display_help() Function
    ---------------------------------------
    Prints the help menu to the standard display
*/
void display_help(void)
{
    printf("This program writes files, reads files, and copies files.\n"
        "When writing to a file, it appends the line buffered input.\n\n"
        "This program works best with valid text based files.\n"
        "Since the program does not check to see if the file is\n"
        "actually a text based file, it may attempt to print\n"
        "garbarge to the display.\n\n");
    printf("%-23s%s\n\n", "h or help", "Prints this help menu to the display.");
    printf("%-23s%s\n", "w or write", "Takes input from the display and writes to [target] file.");
    printf("%-23s%s\n", " ", "When writing a file, input is taken from the display");
    printf("%-23s%s\n", " ", "and written to the [target] file. Output is not echoed.");
    printf("%-23s%s\n\n", " ", "To exit a write session, provide the EOF character.");
    printf("%-23s%s\n", "r or read", "Prints the [target] file to the display.");
    printf("%-23s%s\n", "l or line", "Prints the given line from the [source] file to the display.");
    printf("%-23s%s\n", "c or copy", "Copies the [source] files contents to the [target] file.");
    printf("%-23s%s\n\n", "q to quit", "Exit this application.");
    puts("All files can be given a [target], or [source], name.");
    puts("EOF for Windows is Ctrl+Z");
    puts("EOF for Unix is Ctrl+D\n");
}

/*
    The prompt() Function
    ---------------------------------------
    Introductory prompt for when the program is 
    initially run... is used mainly to clean up
    the code in main() since this is only ever
    used once through-out the program.
*/
void prompt(void)
{
    printf("*************************************************************\n"
        "The Read Line Program\n"
        "*************************************************************\n"
        "The (command)?> display takes only one argument. All subsequent\n"
        "arguments follow suit. You may use a letter, or a word, to be\n" 
        "given as a command. All other options are void. For Example:\n"
        "*************************************************************\n"
        "h[enter] or help[enter] for the Help Display.\n"
        "*************************************************************\n");
}

nanfile.c

/* *********************************
    NANITE FILEs
    ---------------------------
    This file must be linked with the
    nanite.c and nanstring.c files.
    ---------------------------
    This file defines most of the executable
    code predefined by the "nanproto.h" file.
    These functions are used by the "nanfunct.c"
    and "nanite.c" source files.
************************************/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include "./nanproto.h"

/* *********************************
    FILE BASED FUNCTIONS
    --------------------
    these functions operate on files
************************************/

/*
    The known_file_extension() Function
    ---------------------------------------
    IF a known file extension is found,
    return true on success. ELSE return ERROR
    on failure...
*/
int known_file_extension(const char * filename)
{
    enum file type;
    _Bool key_is_found = false;

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

    string_to_lower(file_extension_type);

    if (NULL == file_extension_type)
        return ERROR;

    //find the file type
    for (type = txt; type <= xml; type++)
    {
        if (0 == strcmp(file_extension_type, extension[type]))
        {
            key_is_found = true;
            break;
        }
    }

    //return the keywords position
    if (key_is_found) return true;
    //else keyword was not found
    else return ERROR;
}

/*
    The read_file() Function
    ---------------------------------------
    Prints the given file to the display.
    An ERROR is returned IF anything
    goes awry. ELSE "true" is returned.
    The BUFSIZE macro can be adjusted from
    within the "nanproto.h" file if a
    smaller/larger buffer is desired.
*/
int read_file(const char * filename)
{
    char ch;
    int value;
    long long chars, line = 1;
    long long max_lines = 0;
    FILE * file;

    if (ERROR == known_file_extension(filename))
    {
        fprintf(stderr, "[Invalid Extension Type]: %s\n", filename);
        fprintf(stderr, "You're only allowed to use text files.\n\n");
        return ERROR;
    }

    if ( NULL == (file = fopen(filename, "r")))
    {
        fclose(file);   
        return ERROR;
    }

    while (EOF != (ch = fgetc(file))) 
        if (ch == '\n') max_lines++;

    rewind(file);

    printf("[%ld]: ", line);
    for (line = 2, chars = 0; line <= max_lines; chars++)
    {   
        if (EOF == (ch = fgetc(file))) break;

        if (ch != '\n') fputc(ch, stdout);

        if (ch == '\n') 
        {
            putchar('\n');

            if (chars >= BUFSIZE)
            {
                pause_buffer();
                chars = 0;
            }

            printf("[%ld]: ", line++);
        }
    }
    puts("\n");

    fclose(file);

    return true;
}

/*
    The read_line() Function
    ---------------------------------------
    This function reads one line at a time
    by using the filename and line arguments
    provided. IF the line is found, it is
    printed to standard output.
    The function returns a TRUE value if all
    went well and an ERROR value if anything
    went wrong.
*/
int read_line(const char * filename, long long line)
{
    char ch;
    int value;
    long long current_line, max_lines = 0;
    FILE * file;

    if (ERROR == known_file_extension(filename))
    {
        putchar('\n');
        fprintf(stderr, "[Invalid Extension Type]: %s\n", filename);
        fprintf(stderr, "You're only allowed to use text files.\n\n");
        return ERROR;
    }

    if (NULL == (file = fopen(filename, "r")))
    {
        fclose(file);
        return ERROR;
    }

    while (EOF != (ch = fgetc(file))) 
        if (ch == '\n') max_lines++;

    rewind(file);

    printf("[%ld]: ", line);
    for (current_line = 1; current_line <= max_lines;)
    {
        if (EOF == (ch = fgetc(file))) break;

        if (ch == '\n') current_line++;

        if (current_line == line && ch != '\n') 
            fputc(ch, stdout);
    }
    puts("\n");

    fclose(file);

    return true;
}

/*
    The write_file() Function
    ---------------------------------------
    Writes line buffered input to the target
    file. This function uses the "a" mode, 
    not the "w" mode. Returns an ERROR
    macro if anything went wrong and returns
    true if everything went alright.
    ---------------------------------------
    Even though this function is extremely 
    limiting, its still fun to play with. 
    ---------------------------------------
*/
int write_file(const char * filename)
{
    char ch;
    int value;
    long long chars, line = 1;
    FILE * file;

    if (ERROR == known_file_extension(filename))
    {
        fprintf(stderr, "[Invalid Extension Type]: %s\n", filename);
        fprintf(stderr, "You're only allowed to use text files.\n\n");
        return ERROR;
    }

    if (NULL != (file = fopen(filename, "r")))
    {
        fprintf(stdout, "[Warning!]: The file \"%s\" already exists!\n\n", filename);
        fclose(file);
    } 
    else
    {
        fprintf(stdout, "[Attempting]: to create the file \"%s\"\n\n", filename);
        fclose(file);
    }

    if ( NULL == (file = fopen(filename, "a")))
    {
        fclose(file);
        return ERROR;
    }

    printf("Created the file %s...\n\n", filename);
    printf("[EOF on Newline to Close the File]\n\n");

    printf("[%ld]: ", line);
    for (line = 2, chars = 0; EOF != (ch = fgetc(stdin)); chars++)
    {   
        if (ch != '\n') 
            fputc(ch, file);

        if (ch == '\n') 
        {
            fputc('\n', file);
            printf("[%ld]: ", line++);
        }
    }
    putchar('\n');

    printf("[%lld] characters were written to the file \"%s\"\n", chars, filename);
    printf("[%lld] lines were written to the file \"%s\"\n\n", line, filename);

    fclose(file);   

    return true;
}

/*
    The copy_file() Function
    ---------------------------------------
    Copies the source files contents to the
    target file. This function uses the "w"
    mode, not the "a" mode. Returns an ERROR
    macro if anything went wrong and returns
    true if everything went alright.

*/
int copy_file(const char * source, const char * target)
{
    char ch;
    int value;
    long long chars, line = 1;
    FILE * src, * tar;

    if (ERROR == known_file_extension(source))
    {
        fprintf(stderr, "[Invalid Extension Type]: %s\n", source);
        fprintf(stderr, "You're only allowed to use text files.\n\n");
        return ERROR;
    }

    if (ERROR == known_file_extension(target))
    {
        fprintf(stderr, "[Invalid Extension Type]: %s\n", target);
        fprintf(stderr, "You're only allowed to use text files.\n\n");
        return ERROR;
    }

    if (NULL == (src = fopen(source, "r")))
    {
        fprintf(stdout, "[Error!]: Failed to read the file \"%s\"\n", source);
        fclose(src);
        return ERROR;
    }

    if (NULL == (tar = fopen(target, "w")))
    {
        fprintf(stdout, "[Error!]: Failed to create the file \"%s\"\n", target);
        fclose(tar);
        return ERROR;
    }

    printf("Created the file %s...\n", target);

    for (line = 1, chars = 0; EOF != (ch = fgetc(src)); chars++)
    {   
        fputc(ch, tar);
        if (ch == '\n') line++;
    }

    putchar('\n');

    printf("[%lld] characters were copied to the file \"%s\"\n", chars, target);
    printf("[%lld] lines were copied to the file \"%s\"\n\n", line, target);    

    fclose(src);
    fclose(tar);

    return true;
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Minor: When printing error messages like "Invalid Command: %s\n", consider bracketing the troublesome string to better display any whitespace or linefeed: "Invalid Command: \"%s\"\n". \$\endgroup\$ Jan 25, 2015 at 0:22
  • \$\begingroup\$ @chux Thanks for the tip, I did that and it looks much clearer now. \$\endgroup\$
    – user52380
    Feb 8, 2015 at 1:39

1 Answer 1

4
\$\begingroup\$

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 is wrong:

if ( NULL == (file = fopen(filename, "r")))
{
    fclose(file);
    return ERROR;
}

If file is NULL, don't call fclose on it! I don't know about on other OSes, but this actually segfaults on my OS X machine. That means if I make a small mistake typing a filename, the whole program is brought down. That's a problem.

The General Application and nanite.c

Again, well-written as usual. I have but a few minor complaints.

First of all, why are all these words capitalized? If you're going to go for the informal tone with "Oops!", make it look like a sentence:

puts("Oops! Only Positive Integers Please...");

Also, I found that when I used the write command, it wrote the file fine, but sending EOF caused menu to fail via this path:

fprintf(stderr, "[menu()] -> Failed to successfully store string!");

I'm not sure exactly why that happens, but you might want to look into that.

Overall, this code is quite high-quality. Writing good C code is a feat in and of itself, and this is definitely a great codebase. Is the application itself worth much? Perhaps not, but I'm guessing that wasn't your point to begin with.

\$\endgroup\$
5
  • \$\begingroup\$ Thank you for taking the time to write this review! I laughed at a few points (mainly because I agreed with you in those moments). Warnings: I usually omit the use of Wall or Wextra. Types: This was my biggest issue, so thank you for pointing out the segfault in Unix! I use windows, but set it up with MinGW, Msys, and Cygwin. Blocks: I noticed it's a huge pet peeve, so I'll enclose my blocks from now on while(1) { continue };. Prototypes: You're right. I was drinking when I wrote that. Really. Compliments: Thank you. Helps me realize I'm on the right track. And I'll look in to the EOF sitch. \$\endgroup\$
    – user52380
    Feb 8, 2015 at 1:37
  • \$\begingroup\$ Boolean: As for the 'bool' and 'BOOL #defines', I just really do NOT like the look of '_Bool'... it just looks horribly awkward and throws off the consistency for me. Nanfile.c: '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.' I totally agree. It was just out of ease and laziness that I stopped there. I could have, but chose not to. In more serious programs, I will. \$\endgroup\$
    – user52380
    Feb 8, 2015 at 1:48
  • \$\begingroup\$ @jargonjunkie Oh, I agree, _Bool is horrible. But as I mentioned, stdbool.h includes a typedef that lets you just use bool. \$\endgroup\$ Feb 8, 2015 at 1:54
  • \$\begingroup\$ My MinGW (includes Cygwin as well) doesn't support stdbool.h. my copy of gcc complains otherwise. The file just isn't part of the base library. \$\endgroup\$
    – user52380
    Feb 8, 2015 at 2:07
  • \$\begingroup\$ A thing to note is that on Windows the Microsoft compiler does no support C99 and hence no stdbool.h. Just for maximum portability, this is something to check for. \$\endgroup\$
    – Joakim
    Feb 22, 2015 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.