hi,
i like to change the handling of command line options in layer23 applications, because different layer23 applications require different individual options, and common options also. (e.g. the "mobile" application does not required "--arfcn" option, but "--vty-port". others do not require "--vty-port", but might require a "--gps-device". all apps together require "--socket" and "--gsmtap-ip".)
therefore i like to leave all common options in common/main.c. additional options i like to put in the individual app_*.c files. each options i like to check at the individual app file. if it doesn't exist there, the main.c checks if the option is a common option:
... c = l23_app_handle_options(); if (c == -1) c = getopt_long(argc, argv, "hs:S:a:i:v:d:", long_options, &option_index); if (c == -1) break; ...
an individual help for each application:
l23_app_print_help();
any suggestions or complains?
best regards,
andreas
Hi Andreas,
therefore i like to leave all common options in common/main.c. additional options i like to put in the individual app_*.c files. each options i like to check at the individual app file. if it doesn't exist there, the main.c checks if the option is a common option: ... c = l23_app_handle_options(); if (c == -1) c = getopt_long(argc, argv, "hs:S:a:i:v:d:", long_options, &option_index); if (c == -1) break; ...
To be honest, I don't like the idea of calling two different "instances" of getopt/getopt_long. But that's just me, probably.
What about a app-specific function to call early where the application can register the relevant options:
/* in app.c, called before getopt is started */ l32_app_setup_options(){ l32_add_option('q',"quux",1); /* short, long, hasarg */ l32_add_option('d',"debug",0); }
/* in common/main.c */ l32_add_option(char short,char *long,int hasarg){ /* append short to a global getopts-type-string */ /* append long opts to global table of long opts */ /* mark long-opt index in table (for long-only opts) so that we know to call l32_app_option */ }
And a callback if the option is given:
l32_app_option(char short,char *long,char *val){ if(short == 'q' or !strcmp(long,"quux")){ quux = 42 + atoi(val); } if(short == 'd' or !strcmp(long,"debug")){ debugflag++; } }
I think that makes for even shorter apps with less getopt branching.
Of course it makes it more difficult to handle options where the meaning is position dependant, you have to keep state in global structures then. But we don't want to recreate ffmpeg, do we?
Chris
Hi Christian,
On Tue, Oct 26, 2010 at 05:48:53PM +0200, Christian Vogel wrote:
therefore i like to leave all common options in common/main.c. additional options i like to put in the individual app_*.c files. each options i like to check at the individual app file. if it doesn't exist there, the main.c checks if the option is a common option: ... c = l23_app_handle_options(); if (c == -1) c = getopt_long(argc, argv, "hs:S:a:i:v:d:", long_options, &option_index); if (c == -1) break; ...
To be honest, I don't like the idea of calling two different "instances" of getopt/getopt_long. But that's just me, probably.
I agree.
What about a app-specific function to call early where the application can register the relevant options:
/* in app.c, called before getopt is started */ l32_app_setup_options(){ l32_add_option('q',"quux",1); /* short, long, hasarg */ l32_add_option('d',"debug",0);
yes, I would like that. But don't make it layer23 specific but put it in libosmocore. This way, even individual code parts like the log file handling can register their own options. For the cross-compilation case we can simply make all the functions no-ops.
Of course it makes it more difficult to handle options where the meaning is position dependant, you have to keep state in global structures then. But we don't want to recreate ffmpeg, do we?
No, please don't do it. We've done it for iptables, and it is a nightmare :)
Hi everyone,
I was trying to iterate on that subject again, but I think those are things more easily programmed than discussed. So, my first attempt looks like the attached commit to libosmocore, you can also download it at http://vogel.cx/git/0001-Modular-commandline-parsing.patch .
It uses a few arrays and singly linked lists which get iterated way too often, but I think for commandline parsing it doesn't really matter.
It's used like this:
/* callback function, called for each option given */ int option_callback(struct cmdline_item *p, char *argument){ if(p->shortopt == 'v') verbose++; if(!strncmp(p->longopt,"port")) portnum = atoi(argument); }
struct cmdline_group mygroup = { .name = "Program Options Group", .nitems = 2, .callback = option_callback, .items = { { 'v',"verbose", 0,"Add verbosity." }, { 0 ,"port", 1,"Specify tcp port." }, } };
/* somewhere during initialisation */ cmdline_register_group(&mygroup);
--- configure.in | 1 + include/osmocore/cmdline.h | 37 +++++++++ src/Makefile.am | 2 +- src/cmdline.c | 184 ++++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 2 +- tests/cmdline/Makefile.am | 5 + tests/cmdline/cmdline_test.c | 55 +++++++++++++ 7 files changed, 284 insertions(+), 2 deletions(-) create mode 100644 include/osmocore/cmdline.h create mode 100644 src/cmdline.c create mode 100644 tests/cmdline/Makefile.am create mode 100644 tests/cmdline/cmdline_test.c
diff --git a/configure.in b/configure.in index 30f9d9c..d9c0d30 100644 --- a/configure.in +++ b/configure.in @@ -114,4 +114,5 @@ AC_OUTPUT( tests/sms/Makefile tests/msgfile/Makefile tests/ussd/Makefile + tests/cmdline/Makefile Makefile) diff --git a/include/osmocore/cmdline.h b/include/osmocore/cmdline.h new file mode 100644 index 0000000..e322b45 --- /dev/null +++ b/include/osmocore/cmdline.h @@ -0,0 +1,37 @@ +#ifndef _OSMOCORE_CMDLINE_H +#define _OSMOCORE_CMDLINE_H + +struct cmdline_item { + char shortopt; /* option "-s" */ + char *longopt; /* option "--longopt" */ + int hasarg; /* option has args */ + char *helptext; /* annotate usage */ + union cmdline_item_data { /* random data you want to keep track of */ + char c; + int i; + void *ptr; + } data; +}; + +struct cmdline_group { + char *name; /* name of this group, for usage */ + /* if an option is called, this callback will trigger. + 1st function parameter is the cmdline_item. + 2nc function parameter is the option argument (hasarg != 0). + Return != 0 from the callback to signal an error! */ + int (*callback)(struct cmdline_item*,char *); + + int nitems; /* number of items */ + struct cmdline_group *next; /* used internally */ + struct cmdline_item items[]; /* items in this group */ +}; + +/* add cmdline_group to osmocore command line parser */ +extern void cmdline_register_group(struct cmdline_group *p); + +/* do the actual parsing */ +extern int cmdline_parse(int argc,char **argv,int *optind); + +extern void cmdline_usage(char *argv0); + +#endif diff --git a/src/Makefile.am b/src/Makefile.am index 64310e0..e4fe4a8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,7 +13,7 @@ libosmocore_la_SOURCES = timer.c select.c signal.c msgb.c rxlev_stat.c \ tlv_parser.c bitvec.c comp128.c gsm_utils.c statistics.c \ write_queue.c utils.c rsl.c gsm48.c gsm48_ie.c \ logging.c gsm0808.c rate_ctr.c gsmtap_util.c \ - gprs_cipher_core.c crc16.c panic.c process.c gsm0480.c + gprs_cipher_core.c crc16.c panic.c process.c gsm0480.c cmdline.c
if ENABLE_PLUGIN libosmocore_la_SOURCES += plugin.c diff --git a/src/cmdline.c b/src/cmdline.c new file mode 100644 index 0000000..ddf5747 --- /dev/null +++ b/src/cmdline.c @@ -0,0 +1,184 @@ +/* command line parsing for osmocom applications */ + +/* + * (C) 2010 Christian Vogel vogelchr@vogel.cx + * All Rights Reserved + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + */ + +#include <osmocore/cmdline.h> +#include <osmocore/talloc.h> + +#include <string.h> +#include <unistd.h> +#include <getopt.h> +#include <libgen.h> + +static struct cmdline_group *cmdline_groups = NULL; + +/* + * Find short / long option registered with us. shortopt may be \0 + * and longopt may be NULL, then we ignore this part of an item. + * + * Returns item found. If grpret != NULL the variable pointed to + * will be set to the specific group. + */ +static struct cmdline_item * +cmdline_find_item(char shortopt, + const char *longopt, + struct cmdline_group **grpret) +{ + struct cmdline_group *grp = cmdline_groups; + int i; + + while(grp){ + for(i=0;i<grp->nitems;i++){ + struct cmdline_item *item = &grp->items[i]; + if( (shortopt && shortopt== item->shortopt )|| + (longopt && !strcmp(longopt,item->longopt))){ + if(grpret) + *grpret=grp; + return item; + } + } + grp = grp->next; + } +} + +/* add cmdline_group to osmocore command line parser */ +void +cmdline_register_group(struct cmdline_group *p) +{ + /* insert group into chain */ + struct cmdline_group *ogrp,**last = &cmdline_groups; + struct cmdline_item *oitem,*item; + int i; + + /* just for debugging / finding duplicate options */ + for(i=0;i<p->nitems;i++){ + item = &p->items[i]; + oitem = cmdline_find_item(item->shortopt,item->longopt,&ogrp); + if(oitem){ + fprintf(stderr,"%s: duplicate option registered!", + __FUNCTION__); + if(item->shortopt) + fprintf(stderr," (short: '%c')", + item->shortopt); + if(item->longopt) + fprintf(stderr," (long: "%s")",item->longopt); + fprintf(stderr,"Old group: %s, this group: %s.\n", + p->name,ogrp->name); + } + } + + while(*last) // add as last group + last = &( (*last)->next ); + *last = p; +}; + +void cmdline_usage(char *argv0){ + struct cmdline_group *grp; + char buf[80]; + int i; + + if(!cmdline_groups){ + fprintf(stderr,"Usage: %s arguments\n",basename(argv0)); + return; + } + fprintf(stderr,"Usage: %s [option] arguments\n\n",basename(argv0)); + for(grp=cmdline_groups;grp;grp=grp->next){ + fprintf(stderr,"*** Options in group %s ***\n",grp->name); + for(i=0;i<grp->nitems;i++){ + struct cmdline_item *item = & grp->items[i]; + if(item->shortopt && item->longopt){ + sprintf(buf,"-%c|--%s", + item->shortopt,item->longopt); + } else if(item->shortopt) { + sprintf(buf,"-%c",item->shortopt); + } else { + sprintf(buf,"--%s",item->longopt); + } + if(item->hasarg) + strcat(buf," ARG"); + if(item->helptext) + fprintf(stderr," %-32s %s\n",buf,item->helptext); + else + fprintf(stderr," %s\n",buf); + } + } +} + +int cmdline_parse(int argc,char **argv,int *optind){ + struct cmdline_group *grp; + struct cmdline_item *item; + struct option *opts,*o; + char *optstr,*s; + int i=0,j; + int ret=0; + + /* how many items do we have? */ + for(grp=cmdline_groups;grp;grp=grp->next) + i += grp->nitems; + + /* allocate memory for getopt_long data structures */ + o = opts = talloc_array(NULL,struct option,i+1); + s = optstr = talloc_size(NULL,1+2*i); // worst case + + /* build struct option array and short option string */ + for(grp=cmdline_groups;grp;grp=grp->next){ + for(i=0;i<grp->nitems;i++){ + item = & grp->items[i]; + if(item->longopt){ + o->name = item->longopt; + o->has_arg = item->hasarg; + o->flag = NULL; + o->val = 0; + o++; + } + if(item->shortopt){ + *s++ = item->shortopt; + if(item->hasarg) + *s++ = ':'; + } + } + } + o->name = NULL; + o->has_arg = 0; + o->flag = NULL; + o->val = 0; + *s = '\0'; + + while(-1 != (i=getopt_long(argc,argv,optstr,opts,&j))){ + item = NULL; + if(i==':' || i=='?'){ + ret=-1; // getopt should write error message + goto out; + } + if(i==0) // long option + item = cmdline_find_item(0,opts[j].name,&grp); + else + item = cmdline_find_item(i,NULL,&grp); + if(grp->callback) + grp->callback(item,optarg); + } +out: + talloc_free(opts); + talloc_free(optstr); + return ret; +} + + diff --git a/tests/Makefile.am b/tests/Makefile.am index 0166b4f..ecdc8ad 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,5 +1,5 @@ if ENABLE_TESTS -SUBDIRS = timer sms ussd +SUBDIRS = timer sms ussd cmdline if ENABLE_MSGFILE SUBDIRS += msgfile endif diff --git a/tests/cmdline/Makefile.am b/tests/cmdline/Makefile.am new file mode 100644 index 0000000..3b155e1 --- /dev/null +++ b/tests/cmdline/Makefile.am @@ -0,0 +1,5 @@ +INCLUDES = $(all_includes) -I$(top_srcdir)/include +noinst_PROGRAMS = cmdline_test + +cmdline_test_SOURCES = cmdline_test.c +cmdline_test_LDADD = $(top_builddir)/src/libosmocore.la diff --git a/tests/cmdline/cmdline_test.c b/tests/cmdline/cmdline_test.c new file mode 100644 index 0000000..295b5aa --- /dev/null +++ b/tests/cmdline/cmdline_test.c @@ -0,0 +1,55 @@ +#include <stdlib.h> +#include <unistd.h> +#include <stdio.h> + +#include <osmocore/cmdline.h> + +int option_callback(struct cmdline_item *p,char *argument){ + printf("Option specified:"); + if(p->shortopt) + printf(" -%c",p->shortopt); + if(p->shortopt && p->longopt) + printf(" or"); + if(p->longopt) + printf(" --%s",p->longopt); + if(p->hasarg) + printf(" argument=%s",argument); + printf("\n"); + return 0; +} + +struct cmdline_group group_quux = { + .name = "The QUUX group.", + .nitems = 3, + .callback = option_callback, + .items = { + // -s --long, hasarg, description + { 'a',"aaaah", 1,"The Aaaaaaah option."}, + { 'b',"bleech",0,"The Bleech option."}, + { 'c',"cccc", 1,"The CCCC option."} + } +}; + +struct cmdline_group group_foo = { + .name = "The FOO group.", + .callback = option_callback, + .nitems = 2, + .items = { + { 0 ,"foo", 0,"The foo option."}, + { 'v',NULL , 1,"The v option."} + } +}; + + +int main(int argc,char **argv){ + int optind; + + /* in layer23, these would each go into a different + part of the application, e.g. logging, gsm, ... */ + cmdline_register_group(&group_quux); + cmdline_register_group(&group_foo); + + /* in layer23, this would go to the "common" code */ + if(-1 == cmdline_parse(argc,argv,&optind)) + cmdline_usage(argv[0]); +}
Hi Andreas,
On Tue, Oct 26, 2010 at 02:35:36PM +0200, Andreas.Eversberg wrote:
i like to change the handling of command line options in layer23 applications, because different layer23 applications require different individual options, and common options also. (e.g. the "mobile" application does not required "--arfcn" option, but "--vty-port". others do not require "--vty-port", but might require a "--gps-device". all apps together require "--socket" and "--gsmtap-ip".)
therefore i like to leave all common options in common/main.c. additional options i like to put in the individual app_*.c files. each options i like to check at the individual app file. if it doesn't exist there, the main.c checks if the option is a common option: [...] any suggestions or complains?
looks fine to me. I wonder though, if it actoually would work, I've never tried to do multiple getopt_long() calls with different opstring/longopt arguments.
I always pondered if it would be worth to have a modular commandline option parser as part of libosmocore, as it is something that we could also need in OpenBSC / OsmoBSC / OsmoSGSN & co. In reality, certain protocol modules have a command line argument, or other parts like the logging subsystem inside libosmocore.
baseband-devel@lists.osmocom.org
-
Andreas.Eversberg -
Christian Vogel -
Harald Welte