simple command line parser

Discussion in 'C Programming' started by Greg B, Sep 2, 2003.

  1. Greg B

    Greg B Guest

    Well since getopt() doesn't seem to be compatible with Windows, and
    the free implementation of it for Windows that I found still had some
    annoying restrictions, I thought I'd whip up a simple parser myself.
    Just wanted to see if anyone could provide me with some constructive
    criticism :) any feedback would be greatly appreciated
    -----------------------------------------------------------------------------

    #include "stdio.h"
    #include "string.h"
    #include "stdlib.h"

    #define MAX_OPTION_ARGS 10

    enum optionName {
    Start,
    Stop,
    Remove,
    Install,
    Debug,
    Num_Opts
    };

    typedef struct optionStruct {
    char *name;
    int expNumVal;
    char *val[MAX_OPTION_ARGS];
    int valid;
    } optStruct;

    optStruct optData[Num_Opts] = {
    { "-start", 1 },
    { "-stop", 2 },
    { "-remove", 2 },
    { "-install", 1 },
    { "-debug", 3 }
    };

    int usage(void)
    {
    printf("Usage is as follows...");
    return 0;
    }

    int StartService(char * service_name)
    {
    return 0;
    }

    int main (int argc, char * argv[])
    {
    int a, b, c;
    const char *optstring;
    const struct option *longopts;
    int *longindex;

    for (a = 1; a < argc; a++)
    {
    for (b = 0; b < Num_Opts; b++)
    {
    if (strcmp(argv[a], optData.name) == 0)
    {
    if (a + optData.expNumVal< argc)
    {
    for (c = 0; c < optData.expNumVal; c++)
    {
    optData.val[c] = argv[a + c + 1];
    }
    optData.valid = 1;
    a += optData.expNumVal;
    }
    else
    {
    fprintf(stderr, "ERROR: insufficient number of arguments for
    option %s\n", optData.name);
    usage();
    return 1;
    }

    break;
    }
    }

    if (b == Num_Opts)
    fprintf(stderr, "Warning: unknown option %s (ignored)\n", argv[a]);
    }
    /*
    Sample usage...

    if (optData[Start].valid == 1)
    {
    StartService(optData[Start].val[0]);

    }
    */
    return 0;
    }
     
    Greg B, Sep 2, 2003
    #1
    1. Advertising

  2. (Greg B) wrote in
    <>:

    >Well since getopt() doesn't seem to be compatible with Windows, and
    >the free implementation of it for Windows that I found still had some
    >annoying restrictions, I thought I'd whip up a simple parser myself.
    >Just wanted to see if anyone could provide me with some constructive
    >criticism :) any feedback would be greatly appreciated
    >-----------------------------------------------------------------------------
    >
    >#include "stdio.h"
    >#include "string.h"
    >#include "stdlib.h"
    >
    >#define MAX_OPTION_ARGS 10
    >
    >enum optionName {
    > Start,
    > Stop,
    > Remove,
    > Install,
    > Debug,
    > Num_Opts
    >};
    >
    >typedef struct optionStruct {
    > char *name;
    > int expNumVal;
    > char *val[MAX_OPTION_ARGS];


    That's what I'd call an annoying restriction. :)
    Why not char *val and dynamic memory allocation?

    > int valid;
    >} optStruct;
    >
    >optStruct optData[Num_Opts] = {
    > { "-start", 1 },
    > { "-stop", 2 },
    > { "-remove", 2 },
    > { "-install", 1 },
    > { "-debug", 3 }
    >};

    Nice, you include the dash with every option name. :)

    >
    >int usage(void)
    >{
    > printf("Usage is as follows...");
    > return 0;
    >}
    >
    >int StartService(char * service_name)
    >{
    > return 0;
    >}
    >

    Please, don't use TABs for formatting code you post in usenet -
    this wastes space and garbles the lines.
    <code gently reformatted>
    >int main (int argc, char * argv[])
    >{
    > int a, b, c;
    > const char *optstring;
    > const struct option *longopts;
    > int *longindex;
    >
    > for (a = 1; a < argc; a++)
    > {
    > for (b = 0; b < Num_Opts; b++)
    > {
    > if (strcmp(argv[a], optData.name) == 0)
    > {
    > if (a + optData.expNumVal< argc)
    > {
    > for (c = 0; c < optData.expNumVal; c++)
    > {
    > optData.val[c] = argv[a + c + 1];
    > }
    > optData.valid = 1;
    > a += optData.expNumVal;
    > }
    > else
    > {
    > fprintf(stderr, "ERROR: insufficient number of arguments "
    > "for option %s\n", optData.name);
    > usage();
    > return 1;
    > }
    > break;

    This break has absolutely no chance of beeing executed!

    > }
    > }
    > if (b == Num_Opts)
    > fprintf(stderr, "Warning: unknown option %s (ignored)\n", argv[a]);
    > }
    >/*
    > Sample usage...
    >
    > if (optData[Start].valid == 1)
    > {
    > StartService(optData[Start].val[0]);
    > }
    >*/
    > return 0;
    >}

    Sorry, if my critique is somewhat destructive, but this is one of
    the most complex option parsers I've been looking at. Anyway, if it
    meets your needs, use it, but it might be a good idea to capsule it
    in a separate module in order to keep your main function tidy and
    clean. And you should consider not to make excessive use of global
    variables, as this may lead to unwanted interferences eventually.

    Btw: did you compile it? This is what I got when I did:

    gcc.exe "D:\Temp\ngetopt.c" -o "D:\Temp\ngetopt.exe" -W -Wall
    -Wunreachable-code -ansi -O3 -g3 -I"C:\Programme\DevCpp5b8\include"
    -L"C:\Programme\DevCpp5b8\lib"

    ngetopt.c:25: warning: missing initializer
    ngetopt.c:25: warning: (near initialization for `optData[0].val')
    ngetopt.c:26: warning: missing initializer
    ngetopt.c:26: warning: (near initialization for `optData[1].val')
    ngetopt.c:27: warning: missing initializer
    ngetopt.c:27: warning: (near initialization for `optData[2].val')
    ngetopt.c:28: warning: missing initializer
    ngetopt.c:28: warning: (near initialization for `optData[3].val')
    ngetopt.c:29: warning: missing initializer
    ngetopt.c:29: warning: (near initialization for `optData[4].val')
    ngetopt.c:46: warning: unused variable `optstring'
    ngetopt.c:47: warning: unused variable `longopts'
    ngetopt.c:48: warning: unused variable `longindex'
    ngetopt.c:72: warning: will never be executed
    ngetopt.c:38: warning: unused parameter `service_name'

    If one takes warnings as errors (at least, I do), this is a /bit/
    more than I'd expect from 86 lines of my code (though there are
    no severe warnings, and you can get rid of them very easily).

    I hope my reply wasn't to harsh... :)

    Regards,
    Irrwahn

    --
    When things look dark,
    hold your head high so it can rain up your nose.
     
    Irrwahn Grausewitz, Sep 2, 2003
    #2
    1. Advertising

  3. Greg B

    Eric Sosman Guest

    Irrwahn Grausewitz wrote:
    >
    > (Greg B) wrote:
    >
    > > for (b = 0; b < Num_Opts; b++)
    > > {
    > > if (strcmp(argv[a], optData.name) == 0)
    > > {
    > > if (a + optData.expNumVal< argc)
    > > {
    > > for (c = 0; c < optData.expNumVal; c++)
    > > {
    > > optData.val[c] = argv[a + c + 1];
    > > }
    > > optData.valid = 1;
    > > a += optData.expNumVal;
    > > }
    > > else
    > > {
    > > fprintf(stderr, "ERROR: insufficient number of arguments "
    > > "for option %s\n", optData.name);
    > > usage();
    > > return 1;
    > > }
    > > break;

    > This break has absolutely no chance of beeing executed!


    Why not?

    --
     
    Eric Sosman, Sep 2, 2003
    #3
  4. Eric Sosman <> wrote in <>:
    >Irrwahn Grausewitz wrote:
    >> (Greg B) wrote:
    >> > break;

    >> This break has absolutely no chance of beeing executed!

    >
    > Why not?


    Urgh, sorry, my fault. I'll go looking for my glasses... :)

    --
    Air is water with holes in it.
     
    Irrwahn Grausewitz, Sep 2, 2003
    #4
  5. Greg B

    Alex Guest

    Greg B <> wrote:
    > Well since getopt() doesn't seem to be compatible with Windows, and
    > the free implementation of it for Windows that I found still had some
    > annoying restrictions, I thought I'd whip up a simple parser myself.
    > Just wanted to see if anyone could provide me with some constructive
    > criticism :) any feedback would be greatly appreciated
    > -----------------------------------------------------------------------------


    > #include "stdio.h"
    > #include "string.h"
    > #include "stdlib.h"


    Why are these in double-quotes? It would be more reasonable to
    expect the compiler to look for these headers in their proper
    locations to start with, not in the current directory.

    > #define MAX_OPTION_ARGS 10


    > enum optionName {
    > Start,
    > Stop,
    > Remove,
    > Install,
    > Debug,
    > Num_Opts
    > };


    This has nothing to do with the rest of your code.

    > typedef struct optionStruct {
    > char *name;
    > int expNumVal;
    > char *val[MAX_OPTION_ARGS];
    > int valid;
    > } optStruct;


    If you are going to be spelling out 'Struct' anyway, then why not
    just lose the typedef? You will be saving yourself a shift-key hit.
    If you insist on using the typedef, you do not need the structure
    identifier, since you are not using it.

    > optStruct optData[Num_Opts] = {
    > { "-start", 1 },
    > { "-stop", 2 },
    > { "-remove", 2 },
    > { "-install", 1 },
    > { "-debug", 3 }
    > };


    Why not make the option prefix configurable via a define or somesuch?
    Additionally, you should be initializing 'valid'.

    > int usage(void)
    > {
    > printf("Usage is as follows...");
    > return 0;
    > }


    This is not terribly useful.

    > int StartService(char * service_name)
    > {
    > return 0;
    > }


    Neither is this.

    Here is your reindented code. Still unfitting line length for Usenet,
    I'm afraid.

    > int
    > main (int argc, char *argv[])
    > {
    > int a, b, c;
    > const char *optstring;


    You don't use this.

    > const struct option *longopts;


    What is 'struct option'? You haven't defined it.

    > int *longindex;


    You don't use this.

    > for (a = 1; a < argc; a++)
    > {
    > for (b = 0; b < Num_Opts; b++)
    > {
    > if (strcmp (argv[a], optData.name) == 0)
    > {
    > if (a + optData.expNumVal < argc)
    > {


    This is where it gets really silly. You take the index of the
    argv element you are parsing and add an offset to produce the
    index of an expected supplementary value?

    > for (c = 0; c < optData.expNumVal; c++)
    > {
    > optData.val[c] = argv[a + c + 1];
    > }
    > optData.valid = 1;
    > a += optData.expNumVal;


    Hmmm...

    < snip the rest >

    This is the most complicated and constrained way of getting
    command-line arguments that I have ever seen.

    You need to rethink your design by properly defining the problem
    domain.

    - What kind of options are you going to be parsing?

    - Will there be only boolean options or will you have to handle
    various supplementary types such as strings, integers, floats
    and so on?

    - What will happen if an option has incorrect syntax or a
    a mandatory option is missing?

    - Do you want to copy and paste code for each new program you
    write or do you want to make a universally usable library
    function, which will be configurable by the user?

    To get you started, here is what I think your options structure
    should look like.

    enum opt_t { BOOL, INT, STR /*, etc */ }

    struct option
    {
    char *opt; /* option name */
    opt_t type; /* option type */
    char mandatory; /* is option mandatory ? */
    char *usage; /* usage for this specific option */
    void *value; /* parsed value or null */
    }

    ....only the last element is an output of the parser.

    In your client code, you will populate an array of 'struct
    option' and pass it to your parser along with argc and argv.

    At any time if the parser fails, it will print out a program
    usage based on the 'usage' fields in the array.

    You should also leave it up the client code to define the prefix

    Hope this helps.

    Alex
     
    Alex, Sep 2, 2003
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Ahmed Moustafa

    Parser for command line arguments?

    Ahmed Moustafa, Aug 21, 2003, in forum: Java
    Replies:
    0
    Views:
    395
    Ahmed Moustafa
    Aug 21, 2003
  2. Bernd Oninger
    Replies:
    1
    Views:
    829
    Martin Honnen
    Jul 7, 2004
  3. Boogie El Aceitoso

    Command line parser

    Boogie El Aceitoso, Oct 24, 2003, in forum: C++
    Replies:
    4
    Views:
    670
    J. Campbell
    Oct 25, 2003
  4. Manlio Perillo

    Yet Another Command Line Parser

    Manlio Perillo, Oct 26, 2004, in forum: Python
    Replies:
    9
    Views:
    374
    Manlio Perillo
    Oct 27, 2004
  5. Eric Mahurin
    Replies:
    10
    Views:
    253
    Eric Mahurin
    Sep 14, 2005
Loading...

Share This Page