Small command-line helper tool

Question

I've coded a small command line helper tool for this library I'm working on. The library provides tools for the use of Virtual Texturing on iOS devices (mainly games).

This little command line helper is still pretty much a prototype that I coded as quick as I could, for testing a new file format. Some feedback would be appreciated before I expand it further:

// Virtual Texturing Library:
#include "vt_tool_image.hpp"
#include "vt_tool_pagefile_builder.hpp"
#include "vt_tool_platform_utils.hpp"

// Standard library:
#include <cstdarg>
#include <cstdio>
#include <cstdlib>
#include <cstring>
#include <string>

namespace {

// ======================================================
// Local data:
// ======================================================

vt::tool::PageFileBuilderOptions cmdLineOpts;
std::string inputFile, outputFile;

// ======================================================
// printHelpAndExit()
// ======================================================

void printHelpAndExit()
{
    std::printf("\n"
    "Usage:\n"
    "$ vtmake <input_file> <output_file> [--flags=]\n"
    "\n"
    "Flags accepted:\n"
    "--help           : prints help text with list of commands.\n"
    "--filter         : (str)  type of mipmapping filter: box, tri, quad, cubic, bspline, mitchell, lanczos, sinc, kaiser.\n"
    "--page_size      : (int)  total page size in pixels, including border.\n"
    "--content_size   : (int)  size in pixels of page content, not including border.\n"
    "--border_size    : (int)  size in pixels of the page border.\n"
    "--max_levels     : (int)  max mipmap levels to generate.\n"
    "--flip_v_src     : (bool) flip the source image vertically.\n"
    "--flip_v_tiles   : (bool) flip each individual tile/page vertically.\n"
    "--stop_on_1_mip  : (bool) stop subdividing when mip 0 is reached.\n"
    "--add_debug_info : (bool) print debug text to each page.\n"
    "--dump_images    : (bool) dump each page as an image file (TGA format).\n"
    "--verbose        : (bool) print stuff to STDOUT while running.\n"
    "\n");
    std::exit(0);
}

// ======================================================
// error():
// ======================================================

void error(const char * format, ...)
{
    va_list vaList;
    char buffer[1024];

    va_start(vaList, format);
    std::vsnprintf(buffer, sizeof(buffer), format, vaList);
    va_end(vaList);

    buffer[sizeof(buffer) - 1] = '\0'; // Ensure a null at the end
    throw vt::tool::PageFileBuilderError(buffer);
}

// ======================================================
// parseFilterName():
// ======================================================

vt::tool::FilterType parseFilterName(const char * str)
{
    // Find the value after the '=' sign, if any:
    while ((*str != '=') && (*str != '\0'))
    {
        ++str;
    }
    if (*str == '=') { ++str; }

    if (std::strcmp(str, "box"     ) == 0) { return vt::tool::FilterType::Box;       }
    if (std::strcmp(str, "tri"     ) == 0) { return vt::tool::FilterType::Triangle;  }
    if (std::strcmp(str, "quad"    ) == 0) { return vt::tool::FilterType::Quadratic; }
    if (std::strcmp(str, "cubic"   ) == 0) { return vt::tool::FilterType::Cubic;     }
    if (std::strcmp(str, "bspline" ) == 0) { return vt::tool::FilterType::BSpline;   }
    if (std::strcmp(str, "mitchell") == 0) { return vt::tool::FilterType::Mitchell;  }
    if (std::strcmp(str, "lanczos" ) == 0) { return vt::tool::FilterType::Lanczos;   }
    if (std::strcmp(str, "sinc"    ) == 0) { return vt::tool::FilterType::Sinc;      }
    if (std::strcmp(str, "kaiser"  ) == 0) { return vt::tool::FilterType::Kaiser;    }

    std::printf("WARNING: Unknown filter '%s'! Defaulting to box filter.\n", str);
    return vt::tool::FilterType::Box;
}

// ======================================================
// parseInt():
// ======================================================

int parseInt(const char * str)
{
    // Find the value after the '=' sign, if any:
    while ((*str != '=') && (*str != '\0'))
    {
        ++str;
    }
    if (*str == '=') { ++str; }
    return std::stoi(str);
}

// ======================================================
// parseBool():
// ======================================================

bool parseBool(const char * str)
{
    // Find the value after the '=' sign, if any:
    while ((*str != '=') && (*str != '\0'))
    {
        ++str;
    }
    if (*str == '=') { ++str; }

    if ((std::strcmp(str, "false") == 0) ||
        (std::strcmp(str, "no")    == 0) ||
        (std::strcmp(str, "0")     == 0))
    {
        return false;
    }

    // Assume true for anything else, including an invalid value or an empty string.
    // (results in true for "--flag" with no "=value" part)
    return true;
}

// ======================================================
// startsWith():
// ======================================================

bool startsWith(const char * str, const char * prefix)
{
    const size_t prefixLen = std::strlen(prefix);
    if (prefixLen == 0)
    {
        return false;
    }
    return std::strncmp(str, prefix, prefixLen) == 0;
}

// ======================================================
// parseCmdLine():
// ======================================================

void parseCmdLine(const int argc, const char * argv[])
{
    // Possible "--help" call
    if ((argc == 2) && startsWith(argv[1], "--help"))
    {
        printHelpAndExit();
    }

    // Must have at least argv[0], in_file and out_file
    if (argc < 3)
    {
        error("Not enough arguments!");
    }

    /* argc[0] == "vtmake" (prog name) */
    inputFile  = argv[1];
    outputFile = argv[2];

    for (int i = 3; i < argc; ++i)
    {
        if (startsWith(argv[i], "--help"))
        {
            printHelpAndExit();
        }
        else if (startsWith(argv[i], "--filter"))
        {
            cmdLineOpts.textureFilter = parseFilterName(argv[i]);
        }
        else if (startsWith(argv[i], "--page_size"))
        {
            cmdLineOpts.pageSizePixels = parseInt(argv[i]);
        }
        else if (startsWith(argv[i], "--content_size"))
        {
            cmdLineOpts.pageContentSizePixels = parseInt(argv[i]);
        }
        else if (startsWith(argv[i], "--border_size"))
        {
            cmdLineOpts.pageBorderSizePixels = parseInt(argv[i]);
        }
        else if (startsWith(argv[i], "--max_levels"))
        {
            cmdLineOpts.maxMipLevels = parseInt(argv[i]);
        }
        else if (startsWith(argv[i], "--flip_v_src"))
        {
            cmdLineOpts.flipSourceVertically = parseBool(argv[i]);
        }
        else if (startsWith(argv[i], "--flip_v_tiles"))
        {
            cmdLineOpts.flipTilesVertically = parseBool(argv[i]);
        }
        else if (startsWith(argv[i], "--stop_on_1_mip"))
        {
            cmdLineOpts.stopOn1PageMip = parseBool(argv[i]);
        }
        else if (startsWith(argv[i], "--add_debug_info"))
        {
            cmdLineOpts.addDebugInfoToPages = parseBool(argv[i]);
        }
        else if (startsWith(argv[i], "--dump_images"))
        {
            cmdLineOpts.dumpPageImages = parseBool(argv[i]);
        }
        else if (startsWith(argv[i], "--verbose"))
        {
            cmdLineOpts.stdoutVerbose = parseBool(argv[i]);
        }
        else
        {
            std::printf("WARNING: Unknown command line argument: '%s'\n", argv[i]);
        }
    }

    if (cmdLineOpts.stdoutVerbose)
    {
        std::printf("Input  file: \"%s\"\n", inputFile.c_str());
        std::printf("Output file: \"%s\"\n", outputFile.c_str());
        cmdLineOpts.printSelf();
    }
}

// ======================================================
// runPageFileBuilder():
// ======================================================

void runPageFileBuilder()
{
    if (inputFile.empty())
    {
        error("No input filename!");
    }
    if (outputFile.empty())
    {
        error("No output filename!");
    }

    vt::tool::PageFileBuilder pageFileBuilder(inputFile, outputFile, cmdLineOpts);
    pageFileBuilder.generatePageFile();
    std::printf("Done!\n");
}

} // namespace {}

// ======================================================
// main():
// ======================================================

int main(int argc, const char * argv[])
{
    try
    {
        parseCmdLine(argc, argv);
        runPageFileBuilder();
        return 0;
    }
    catch (std::exception & e)
    {
        std::printf("ERROR: %s\n", e.what());
        return -1;
    }
}

The main purpose of this code, as you can see, is to parse and validate command line args. The heavy work is then done by the library.

It looks very C-ish, I'll admit. Since I wrote it quickly for testing, I didn't bother much. Also, command args validation is still pretty weak. I might consider refactoring it into a class and using less char* and more std::string.

Answer 1

Standard conventions indicate that the input and output files come last.
Flags are modifiers that appear first.

Both input and output should be optional and if not supplied be replaced with standard in/out. Note the - by itself instead of a flag should indicate standard input/output depending on its position.

Thus I would write it like this:

vtmake [--flags=] [<input_file> [<output_file>]]

Example Usage:

vtmake                   // Reads standard input writes to standard output
vtmake X                 // Reads X writes to standard output
vtmake - Y               // Reads standard input writes to Y
vtmake X Y               // Reads X writes to Y

There are also some very good command line parsing utilities (so you don't have to write boilerplate code with potential mistakes). When somebody has gone to the effort of writing and debugging for you you should probably use them.

http://www.gnu.org/software/libc/manual/html_node/Parsing-Program-Arguments.html

Answer 2

• Do not hardcode vtmake in printHelpAndExit Print argv[0] instead.

• Each parseWhatever function has its own copy of code finding the =. This must be factored out into a function; even better, use strchr.

  I would also recommend to restructure code to let parseCmdLine find the =.

• The loop in parseCmdLine contains too much code. I would rather have a map of options to parsing functions. Similarly, I'd have a map of filter names to vt::tool::FilterType values.

• The overall command line structure is quite unconventional. Typically positional parameters follow options. Also it is very useful to make filenames optional as well: such tool can be then be used in a pipeline.

• I don't see the need for cmdLineOpts, inputFile, outputFile to exist in a namespace scope. I recommend to define them in main and pass them to interested parties as parameters.

• Finally, why not just use getopt?

• A side note:

  vt::tool::PageFileBuilder pageFileBuilder(inputFile, outputFile, cmdLineOpts);
  pageFileBuilder.generatePageFile();
  

  doesn't seem right. The PageFileBuilder looks like an implementation detail of generatePageFile, and should not be exposed to client.

Answer 3

For organizational purposes, I can see putting some of the reusable parsing stuff into a separate namespace (with it's own header/cxx file) if you think you might need it for another project later. Overall this looks really nice though.