ru******@hotmail.com (Greg B) wrote in
<f5**************************@posting.google.com >:
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[b].name) == 0)
{
if (a + optData[b].expNumVal< argc)
{
for (c = 0; c < optData[b].expNumVal; c++)
{
optData[b].val[c] = argv[a + c + 1];
}
optData[b].valid = 1;
a += optData[b].expNumVal;
}
else
{
fprintf(stderr, "ERROR: insufficient number of arguments "
"for option %s\n", optData[b].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.
