process CLI args with getopt

[msimerson]

• Replaced the manual for loop with getopt(argc, argv, "f:d:n")
  • options can now appear in any order
  • unknown flags get a proper error
• After getopt, argv[optind] is the positional argument (key or key=value), so arg_string is set from there
• Replaced the argc == 3 heuristic for "no key given" with arg_string == NULL, which is accurate regardless of option order
• Improved usage string to show full synopsis
• fix 'syconf' typos in docs

[JohnKaul]

NOTE: I'm at work (no access to a compiler and all that stuff) so I can only speak on a high level. So, please feel free to answer at your leisure.

• Replaced the manual for loop with getopt(argc, argv, "f:d:n")
  * options can now appear in any order
  * unknown flags get a proper error
  ...

Your list of reasons gave me pause because of the implications of using getopt() and the reasons listed were a bit contradictory (-i.e. a "were you aware" type of question to follow).

The current call syntax being:
sysconf [-n] -f <FILE> <KEY>

And the current (overly) simple loop construct supports this:

// -Parse the command line options.
for (int i = 0; i < argc; i++) {
  if (argv[i] && strlen(argv[i]) > 1) {
    if (argv[i][0] == '-' && argv[i][1] == 'f') { file_string = argv[++i]; }
    if (argv[i][0] == '-' && argv[i][1] == 'd') { default_string = argv[++i]; }
    if (argv[i][0] == '-' && argv[i][1] == 'n') { keyvalue_output = 1; }
    if (argv[i][0] != '-') { arg_string = argv[i]; }
  }
}

But your statement of 'allowing items in *any* order' is making me wonder if I should ask for clarification because if we were to move one line up like this:

// -Parse the command line options.
for (int i = 0; i < argc; i++) {
  if (argv[i] && strlen(argv[i]) > 1) {
    if (argv[i][0] != '-') { arg_string = argv[i]; }
    if (argv[i][0] == '-' && argv[i][1] == 'f') { file_string = argv[++i]; }
    if (argv[i][0] == '-' && argv[i][1] == 'd') { default_string = argv[++i]; }
    if (argv[i][0] == '-' && argv[i][1] == 'n') { keyvalue_output = 1; }
  }
}

The program can be called in a lot of different ways, like:

sysconf <KEY> [-n] -f <FILE>
sysconf -f <FILE> <KEY> [-n]
sysconf [-n] -f <FILE> <KEY>

Where as using getopt() will confine the order of options to: sysconf [-n] -f <FILE> <KEY>. Again, I'm speaking without access to a compiler but I just wanted to toss this bit of information out there just in case you were not aware (even at the risk you could very well be a programming wizard--understood this before making the change--and I just put my foot in my mouth).

And, of course my simple loop construct doesn't address the second reason ('proper error') because my simple loop construct just ignores the unknown (and this alone could probably be reason enough to move to getopt()). But if true 'flexible option order' something that is desirable then we can achieve this with a one-line change and I wanted your opinion on what you think is better because I believe I remember you saying you used this tool in automation (with scripts I assume) where flexible option order isn't as critical as is when supporting end users typing on the CLI. -i.e. would it be "better" in a more "stricter" or "looser" option order pattern for this type of tool?

At any rate: thank you. I see you found a few other errors (spelling and whatnot). Spelling errors in my stuff is probably enough to keep several people busy full time. :)

[msimerson]

Where as using getopt() will confine the order of options to:

I'm not following your line of reasoning here, but I've added ./test/getopt.sh to this PR now which tests that identical results are achieved no matter which way the arguments are specified in. I also updated the docs accordingly, and extended the makefile so that make test runs the new tests.

[JohnKaul]

Please do not go to too much trouble on my account. But I was only wondering if arguments in *any* position is important.

When using a POSIX/BSD getopt(3) you cannot place a non-optional argument first or before optional ones (so, in your script, create an option that places the "key=value" before the "-ndf" options). However, I have created a few C files you can test with too if that makes it easier to decide.

If I remember right, GNU getopt() allows arguments in *any* position (unlike BSD) so, if we use the custom loop version, the utility can be used in the same on both types of systems (or with either compiler/lib).

It doesn't bother me which is used but at the same time we don't need all the features of getopt() (like option chaining and stuff) either (but I do like constancy!).

test_getopt.c

#include <unistd.h>
#include <stdio.h>

/**
 * This code should act as below because POSIX getopt(3) acts as:
 *  "
 *    When all options have been processed (i.e., up to the first
 *    non-option argument), getopt() returns -1.
 *  "
 * EXAMPLE RUN:
 *      [test]cc -o test_getopt test_getopt.c
 *      [test]./test_getopt -n -f file.in -d default.in key=value
 *       filestring: file.in
 *       defaultstring: default.in
 *       argstring: key=value
 *       showkey: 1
 *      [test]./test_getopt key=value -n -f file.in -d default.in
 *       filestring: (null)
 *       defaultstring: (null)
 *       argstring: key=value
 *       showkey: 0
 *      [test]
 */
int main(int argc, char *argv[]) {
  char *file_string = NULL;                             /* Used to store config file name */
  char *default_string = NULL;                          /* Used to store default config file name. */
  char *arg_string = NULL;                              /* Used to store the argument string. */
  int keyvalue_output = 0;

  int opt;
  while ((opt = getopt(argc, argv, "f:d:n")) != -1) {
    switch (opt) {
      case 'f': file_string    = optarg; break;
      case 'd': default_string = optarg; break;
      case 'n': keyvalue_output = 1;     break;
      default:
                fprintf(stderr, "Usage: %s -f <configuration file> [-d <defaults file>] [-n] [key[=value]]\n", argv[0]);
    }
  }

  if (optind < argc) arg_string = argv[optind];

  printf(" filestring: %s\n defaultstring: %s\n argstring: %s\n showkey: %d\n",
      file_string,
      default_string,
      arg_string,
      keyvalue_output);
}

test_customargparser.c

#include <unistd.h>
#include <stdio.h>
#include <string.h>

/**
 * EXAMPLE RUN:
 *      [test]cc -o test_customargparser test_customargparser.c
 *      [test]./test_customargparser -n -f file.in -d default.in key=value
 *       filestring: file.in
 *       defaultstring: default.in
 *       argstring: -n
 *       showkey: 1
 *      [test]./test_customargparser key=value -n -f file.in -d default.in
 *       filestring: file.in
 *       defaultstring: default.in
 *       argstring: key=value
 *       showkey: 1
 *      [test]
 */
int main(int argc, char *argv[]) {
  char *file_string = NULL;                             /* Used to store config file name */
  char *default_string = NULL;                          /* Used to store default config file name. */
  char *arg_string = NULL;                              /* Used to store the argument string. */
  int keyvalue_output = 0;

  // -Parse the command line options.
  for (int i = 0; i < argc; i++) {
    if (argv[i] && strlen(argv[i]) > 1) {
      if (argv[i][0] != '-') { arg_string = argv[i]; }
      if (argv[i][0] == '-' && argv[i][1] == 'f') { file_string = argv[++i]; }
      if (argv[i][0] == '-' && argv[i][1] == 'd') { default_string = argv[++i]; }
      if (argv[i][0] == '-' && argv[i][1] == 'n') { keyvalue_output = 1; }
    }
  }

  printf(" filestring: %s\n defaultstring: %s\n argstring: %s\n showkey: %d\n",
      file_string,
      default_string,
      arg_string,
      keyvalue_output);
}