26 Oct
2010
26 Oct
'10
2:35 p.m.
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
26 Oct
26 Oct
5:48 p.m.
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
5:50 p.m.
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.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
7:44 p.m.
Hi Christian,
On Tue, Oct 26, 2010 at 05:48:53PM +0200, Christian Vogel wrote:
I agree.
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);
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 :)
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
27 Oct
27 Oct
10:28 p.m.
New subject: [PATCH] Modular Commandline Parsing [Re: command line options for layer23 apps]
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(a)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]);
+}
--
1.6.3.3
4953
days inactive
4953
days old
baseband-devel@lists.osmocom.org
4 comments
3 participants
participants (3)
-
Andreas.Eversberg -
Christian Vogel -
Harald Welte