From be4bac9483bfd4fa8cbe1cba2c61d7f85e83b36c Mon Sep 17 00:00:00 2001 From: Tobias Oetiker Date: Mon, 29 Jun 2015 17:44:41 +0200 Subject: [PATCH] rrd_xport thread safety patch part 1 * introduce optparse as a replacement for getopt * switch rrd_xport to optparse to make it threadsafe * pass absolute argument position to rrd_graph_script to make it getopt independent --- src/Makefile.am | 3 +- src/optparse.c | 257 ++++++++++++++++++++++++++++++++++++++++++++++++ src/optparse.h | 102 +++++++++++++++++++ src/rrd_graph.c | 2 +- src/rrd_xport.c | 112 ++++++++++----------- 5 files changed, 415 insertions(+), 61 deletions(-) create mode 100644 src/optparse.c create mode 100644 src/optparse.h diff --git a/src/Makefile.am b/src/Makefile.am index 6b77ea1f..c30eb2b7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -55,6 +55,7 @@ if BUILD_RRDGRAPH RRD_C_FILES += rrd_graph.c \ rrd_graph_helper.c \ rrd_xport.c \ + optparse.c \ rrd_gfx.c \ pngsize.c endif @@ -71,7 +72,7 @@ noinst_HEADERS = \ rrd_snprintf.h \ rrd_getopt.h rrd_parsetime.h \ rrd_config_bottom.h rrd_i18n.h \ - rrd_format.h rrd_tool.h rrd_xport.h rrd.h rrd_rpncalc.h \ + rrd_format.h rrd_tool.h rrd_xport.h optparse.h rrd.h rrd_rpncalc.h \ rrd_hw.h rrd_hw_math.h rrd_hw_update.h \ rrd_restore.h rrd_create.h \ fnv.h rrd_graph.h \ diff --git a/src/optparse.c b/src/optparse.c new file mode 100644 index 00000000..8889b947 --- /dev/null +++ b/src/optparse.c @@ -0,0 +1,257 @@ +#include +#include "optparse.h" + +#define opterror(options, format, args...) \ + snprintf(options->errmsg, sizeof(options->errmsg), format, args); + +void optparse_init(struct optparse *options, char **argv) +{ + options->argv = argv; + options->permute = 1; + options->optind = 1; + options->subopt = 0; + options->optarg = NULL; + options->errmsg[0] = '\0'; +} + +static inline int +is_dashdash(const char *arg) +{ + return arg != NULL && arg[0] == '-' && arg[1] == '-' && arg[2] == '\0'; +} + +static inline int +is_shortopt(const char *arg) +{ + return arg != NULL && arg[0] == '-' && arg[1] != '-' && arg[1] != '\0'; +} + +static inline int +is_longopt(const char *arg) +{ + return arg != NULL && arg[0] == '-' && arg[1] == '-' && arg[2] != '\0'; +} + +static void +permute(struct optparse *options, int index) +{ + char *nonoption = options->argv[index]; + for (int i = index; i < options->optind - 1; i++) + options->argv[i] = options->argv[i + 1]; + options->argv[options->optind - 1] = nonoption; +} + +static enum optparse_argtype +argtype(const char *optstring, char c) +{ + if (c == ':') + return -1; + for (; *optstring && c != *optstring; optstring++); + if (!*optstring) + return -1; + enum optparse_argtype count = OPTPARSE_NONE; + if (optstring[1] == ':') + count += optstring[2] == ':' ? 2 : 1; + return count; +} + +int optparse(struct optparse *options, const char *optstring) +{ + options->errmsg[0] = '\0'; + options->optopt = 0; + options->optarg = NULL; + char *option = options->argv[options->optind]; + if (option == NULL) { + return -1; + } else if (is_dashdash(option)) { + options->optind++; // consume "--" + return -1; + } else if (!is_shortopt(option)) { + if (options->permute) { + int index = options->optind; + options->optind++; + int r = optparse(options, optstring); + permute(options, index); + options->optind--; + return r; + } else { + return -1; + } + } + option += options->subopt + 1; + options->optopt = option[0]; + int type = argtype(optstring, option[0]); + char *next = options->argv[options->optind + 1]; + switch (type) { + case -1: + opterror(options, "invalid option -- '%c'", option[0]); + options->optind++; + return '?'; + case OPTPARSE_NONE: + if (option[1]) { + options->subopt++; + } else { + options->subopt = 0; + options->optind++; + } + return option[0]; + case OPTPARSE_REQUIRED: + options->subopt = 0; + options->optind++; + if (option[1]) { + options->optarg = option + 1; + } else if (next != NULL) { + options->optarg = next; + options->optind++; + } else { + opterror(options, "option requires an argument -- '%c'", option[0]); + options->optarg = NULL; + return '?'; + } + return option[0]; + case OPTPARSE_OPTIONAL: + options->subopt = 0; + options->optind++; + if (option[1]) + options->optarg = option + 1; + else + options->optarg = NULL; + return option[0]; + } + return 0; +} + +char *optparse_arg(struct optparse *options) +{ + options->subopt = 0; + char *option = options->argv[options->optind]; + if (option != NULL) + options->optind++; + return option; +} + +static inline int +longopts_end(const struct optparse_long *longopts, int i) +{ + return !longopts[i].longname && !longopts[i].shortname; +} + +static size_t +optstring_length(const struct optparse_long *longopts) +{ + int length = 0; + for (int i = 0; !longopts_end(longopts, i); i++, length++) + length += longopts[i].argtype; + return length + 1; +} + +static void +optstring_from_long(const struct optparse_long *longopts, char *optstring) +{ + char *p = optstring; + for (int i = 0; !longopts_end(longopts, i); i++) { + if (longopts[i].shortname) { + *p++ = longopts[i].shortname; + for (int a = 0; a < longopts[i].argtype; a++) + *p++ = ':'; + } + } + *p = '\0'; +} + +/* Unlike strcmp(), handles options containing "=". */ +static int +longopts_match(const char *longname, const char *option) +{ + if (longname == NULL) + return 0; + const char *a = option, *n = longname; + for (; *a && *n && *a != '='; a++, n++) + if (*a != *n) + return 0; + return *n == '\0' && (*a == '\0' || *a == '='); +} + +/* Return the part after "=", or NULL. */ +static const char * +longopts_arg(const char *option) +{ + for (; *option && *option != '='; option++); + if (*option == '=') + return option + 1; + else + return NULL; +} + +static int +long_fallback(struct optparse *options, + const struct optparse_long *longopts, + int *longindex) +{ + char optstring[optstring_length(longopts)]; + optstring_from_long(longopts, optstring); + int result = optparse(options, optstring); + if (longindex != NULL) { + *longindex = -1; + if (result != -1) + for (int i = 0; !longopts_end(longopts, i); i++) + if (longopts[i].shortname == options->optopt) + *longindex = i; + } + return result; +} + +int +optparse_long(struct optparse *options, + const struct optparse_long *longopts, + int *longindex) +{ + char *option = options->argv[options->optind]; + if (option == NULL) { + return -1; + } else if (is_shortopt(option)) { + return long_fallback(options, longopts, longindex); + } else if (!is_longopt(option)) { + if (options->permute) { + int index = options->optind; + options->optind++; + int r = optparse_long(options, longopts, longindex); + permute(options, index); + options->optind--; + return r; + } else { + return -1; + } + } + + /* Parse as long option. */ + options->errmsg[0] = '\0'; + options->optopt = 0; + options->optarg = NULL; + option += 2; // skip "--" + options->optind++; + for (int i = 0; !longopts_end(longopts, i); i++) { + const char *name = longopts[i].longname; + if (longopts_match(name, option)) { + if (longindex) + *longindex = i; + options->optopt = longopts[i].shortname; + const char *arg = longopts_arg(option); + if (longopts[i].argtype == OPTPARSE_NONE && arg != NULL) { + opterror(options, "option takes no arguments -- '%s'", name); + return '?'; + } if (arg != NULL) { + options->optarg = arg; + } else if (longopts[i].argtype == OPTPARSE_REQUIRED) { + options->optarg = options->argv[options->optind++]; + if (options->optarg == NULL) { + opterror(options, "option requires argument -- '%s'", name); + return '?'; + } + } + return options->optopt; + } + } + opterror(options, "invalid option -- '%s'", option); + return '?'; +} diff --git a/src/optparse.h b/src/optparse.h new file mode 100644 index 00000000..3e70dc04 --- /dev/null +++ b/src/optparse.h @@ -0,0 +1,102 @@ +#ifndef OPTPARSE_H +#define OPTPARSE_H + +/** + * Optparse -- portable, reentrant, embeddable, getopt-like option parser + * + * The POSIX getopt() option parser has three fatal flaws. These flaws + * are solved by Optparse. + * + * 1) Parser state is stored entirely in global variables, some of + * which are static and inaccessible. This means only one thread can + * use getopt(). It also means it's not possible to recursively parse + * nested sub-arguments while in the middle of argument parsing. + * Optparse fixes this by storing all state on a local struct. + * + * 2) The POSIX standard provides no way to properly reset the parser. + * This means for portable code that getopt() is only good for one + * run, over one argv with one optstring. It also means subcommand + * options cannot be processed with getopt(). Most implementations + * provide a method to reset the parser, but it's not portable. + * Optparse provides an optparse_arg() function for stepping over + * subcommands and continuing parsing of options with another + * optstring. The Optparse struct itself can be passed around to + * subcommand handlers for additional subcommand option parsing. A + * full reset can be achieved by with an additional optparse_init(). + * + * 3) Error messages are printed to stderr. This can be disabled with + * opterr, but the messages themselves are still inaccessible. + * Optparse solves this by writing an error message in its errmsg + * field. The downside to Optparse is that this error message will + * always be in English rather than the current locale. + * + * Optparse should be familiar with anyone accustomed to getopt(), and + * it could be a nearly drop-in replacement. The optstring is the same + * and the fields have the same names as the getopt() global variables + * (optarg, optind, optopt). + * + * Optparse also supports GNU-style long options with optparse_long(). + * The interface is slightly different and simpler than getopt_long(). + * + * By default, argv is permuted as it is parsed, moving non-option + * arguments to the end. This can be disabled by setting the `permute` + * field to 0 after initialization. + */ + +struct optparse { + char **argv; + int permute; + int optind; + int optopt; + const char *optarg; + char errmsg[48]; + int subopt; +}; + +enum optparse_argtype { OPTPARSE_NONE, OPTPARSE_REQUIRED, OPTPARSE_OPTIONAL }; + +struct optparse_long { + const char *longname; + int shortname; + enum optparse_argtype argtype; +}; + +/** + * Initializes the parser state. + */ +void optparse_init(struct optparse *options, char **argv); + +/** + * Read the next option in the argv array. + * @param optstring a getopt()-formatted option string. + * @return the next option character, -1 for done, or '?' for error + * + * Just like getopt(), a character followed by a colon means no + * argument. One colon means the option has a required argument. Two + * colons means the option takes an optional argument. + */ +int optparse(struct optparse *options, const char *optstring); + +/** + * Handles GNU-style long options in addition to getopt() options. + * This works a lot like GNU's getopt_long(). The last option in + * longopts must be all zeros, marking the end of the array. The + * longindex argument may be NULL. + */ +int +optparse_long(struct optparse *options, + const struct optparse_long *longopts, + int *longindex); + +/** + * Used for stepping over non-option arguments. + * @return the next non-option argument, or -1 for no more arguments + * + * Argument parsing can continue with optparse() after using this + * function. That would be used to parse the options for the + * subcommand returned by optparse_arg(). This function allows you to + * ignore the value of optind. + */ +char *optparse_arg(struct optparse *options); + +#endif diff --git a/src/rrd_graph.c b/src/rrd_graph.c index 9e5912e2..47c4dcb0 100644 --- a/src/rrd_graph.c +++ b/src/rrd_graph.c @@ -4577,7 +4577,7 @@ rrd_info_t *rrd_graph_v( im.graphfile[0] = '\0'; } - rrd_graph_script(argc, argv, &im, 1); + rrd_graph_script(argc, argv, &im, optind+1); if (rrd_test_error()) { rrd_info_free(im.grinfo); diff --git a/src/rrd_xport.c b/src/rrd_xport.c index 629b118e..f714e8a7 100644 --- a/src/rrd_xport.c +++ b/src/rrd_xport.c @@ -1,7 +1,7 @@ /**************************************************************************** * RRDtool 1.GIT, Copyright by Tobi Oetiker **************************************************************************** - * rrd_xport.c export RRD data + * rrd_xport.c export RRD data ****************************************************************************/ #include @@ -12,6 +12,7 @@ #include "rrd_xport.h" #include "unused.h" #include "rrd_client.h" +#include "optparse.h" #if defined(_WIN32) && !defined(__CYGWIN__) && !defined(__CYGWIN32__) #include @@ -63,7 +64,7 @@ int rrd_xport( time_t *start, time_t *end, /* which time frame do you want ? * will be changed to represent reality */ - unsigned long *step, /* which stepsize do you want? + unsigned long *step, /* which stepsize do you want? * will be changed to represent reality */ unsigned long *col_cnt, /* number of data columns in the result */ char ***legend_v, /* legend entries */ @@ -73,21 +74,20 @@ int rrd_xport( time_t start_tmp = 0, end_tmp = 0; rrd_time_value_t start_tv, end_tv; char *parsetime_error = NULL; - - struct option long_options[] = { - {"start", required_argument, 0, 's'}, - {"end", required_argument, 0, 'e'}, - {"maxrows", required_argument, 0, 'm'}, - {"step", required_argument, 0, 261}, - {"enumds", no_argument, 0, 262}, /* these are handled in the frontend ... */ - {"json", no_argument, 0, 263}, /* these are handled in the frontend ... */ - {"daemon", required_argument, 0, 'd'}, - {0, 0, 0, 0} + struct optparse options; + optparse_init(&options, argv); + + struct optparse_long longopts[] = { + {"start", 's', OPTPARSE_REQUIRED}, + {"end", 'e', OPTPARSE_REQUIRED}, + {"maxrows",'m', OPTPARSE_REQUIRED}, + {"step", 261, OPTPARSE_REQUIRED}, + {"enumds", 262, OPTPARSE_NONE}, + {"json", 263, OPTPARSE_NONE}, /* these are handled in the frontend ... */ + {"daemon", 'd', OPTPARSE_REQUIRED}, + {0} }; - optind = 0; - opterr = 0; /* initialize getopt */ - rrd_graph_init(&im); rrd_parsetime("end-24h", &start_tv); @@ -96,18 +96,12 @@ int rrd_xport( int enumds=0; int json=0; - while (1) { - int option_index = 0; - int opt; - - opt = getopt_long(argc, argv, "s:e:m:d:", long_options, &option_index); - - if (opt == EOF) - break; + int opt; + while ((opt = optparse_long(&options,longopts,NULL)) != -1){ switch (opt) { case 261: - im.step = atoi(optarg); + im.step = atoi(options.optarg); break; case 262: enumds=1; @@ -116,19 +110,19 @@ int rrd_xport( json=1; break; case 's': - if ((parsetime_error = rrd_parsetime(optarg, &start_tv))) { + if ((parsetime_error = rrd_parsetime(options.optarg, &start_tv))) { rrd_set_error("start time: %s", parsetime_error); return -1; } break; case 'e': - if ((parsetime_error = rrd_parsetime(optarg, &end_tv))) { + if ((parsetime_error = rrd_parsetime(options.optarg, &end_tv))) { rrd_set_error("end time: %s", parsetime_error); return -1; } break; case 'm': - im.xsize = atol(optarg); + im.xsize = atol(options.optarg); if (im.xsize < 10) { rrd_set_error("maxrows below 10 rows"); return -1; @@ -143,7 +137,7 @@ int rrd_xport( return (-1); } - im.daemon_addr = strdup(optarg); + im.daemon_addr = strdup(options.optarg); if (im.daemon_addr == NULL) { rrd_set_error("strdup error"); @@ -153,7 +147,7 @@ int rrd_xport( } case '?': - rrd_set_error("unknown option '%s'", argv[optind - 1]); + rrd_set_error("%s: %s", argv[0], options.errmsg); return -1; } } @@ -178,7 +172,7 @@ int rrd_xport( im.end = end_tmp; im.step = max((long) im.step, (im.end - im.start) / im.xsize); - rrd_graph_script(argc, argv, &im, 0); + rrd_graph_script(argc, argv, &im, options.optind); if (rrd_test_error()) { im_free(&im); return -1; @@ -206,7 +200,7 @@ int rrd_xport( if (json) { flags|=1; } if (enumds) { flags|=4; } stringbuffer_t buffer={0,0,NULL,stdout}; - rrd_xport_format_xmljson(flags,&buffer,&im, + rrd_xport_format_xmljson(flags,&buffer,&im, *start, *end, *step, *col_cnt, *legend_v, *data); @@ -223,7 +217,7 @@ int rrd_xport_fn( time_t *start, time_t *end, /* which time frame do you want ? * will be changed to represent reality */ - unsigned long *step, /* which stepsize do you want? + unsigned long *step, /* which stepsize do you want? * will be changed to represent reality */ unsigned long *col_cnt, /* number of data columns in the result */ char ***legend_v, /* legend entries */ @@ -238,7 +232,7 @@ int rrd_xport_fn( unsigned long xport_counter = 0; int *ref_list; long *step_list; - long *step_list_ptr; + long *step_list_ptr; char **legend_list; @@ -328,12 +322,12 @@ int rrd_xport_fn( ++j; } } - *step_list_ptr=0; + *step_list_ptr=0; /* find a common step */ *step = lcd(step_list); /* printf("step: %lu\n",*step); */ free(step_list); - + *start = im->start - im->start % (*step); if ( im->start > *start ) { *start = *start + *step; @@ -386,7 +380,7 @@ int rrd_graph_xport(image_desc_t *im) { char **legend_v=NULL; rrd_value_t *data=NULL; /* initialize buffer */ - stringbuffer_t buffer={0,0,NULL,NULL}; + stringbuffer_t buffer={0,0,NULL,NULL}; /* check if we have a supported ggraph format */ switch (im->graph_type) { @@ -464,12 +458,12 @@ int rrd_graph_xport(image_desc_t *im) { /* now do the cleanup */ if (buffer.file) { - fclose(buffer.file); buffer.file=NULL; + fclose(buffer.file); buffer.file=NULL; im->rendered_image_size=0; im->rendered_image=NULL; } else { im->rendered_image_size=buffer.len; - im->rendered_image=buffer.data; + im->rendered_image=buffer.data; } /* and print stuff */ @@ -480,19 +474,19 @@ int addToBuffer(stringbuffer_t * sb,char* data,size_t len) { /* if len <= 0 we assume a string and calculate the length ourself */ if (len<=0) { len=strlen(data); } /* if we have got a file, then take the shortcut */ - if (sb->file) { + if (sb->file) { sb->len+=len; - fwrite(data,len,1,sb->file); - return 0; + fwrite(data,len,1,sb->file); + return 0; } /* if buffer is 0, then initialize */ - if (! sb->data) { + if (! sb->data) { /* make buffer a multiple of 8192 */ sb->allocated+=8192; - sb->allocated-=(sb->allocated%8192); + sb->allocated-=(sb->allocated%8192); /* and allocate it */ sb->data = (unsigned char *) malloc(sb->allocated); - if (! sb->data) { + if (! sb->data) { rrd_set_error("malloc issue"); return 1; } @@ -598,7 +592,7 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t /* define the time format */ char* timefmt=NULL; - /* unfortunatley we have to do it this way, + /* unfortunatley we have to do it this way, as when no --x-graph argument is given, then the xlab_user is not in a clean state (e.g. zero-filled) */ if (im->xlab_user.minsec!=-1.0) { timefmt=im->xlab_user.stst; } @@ -626,7 +620,7 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t else { addToBuffer(buffer,"{ \"about\": \"RRDtool graph JSON output\",\n \"meta\": {\n",0); } - + /* calculate start time */ if (timefmt) { struct tm loc; @@ -678,7 +672,7 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t snprintf(buf,sizeof(buf)," <%s>%lu\n",META_COLS_TAG,col_cnt,META_COLS_TAG); addToBuffer(buffer,buf,0); } - + /* start legend */ if (json){ snprintf(buf,sizeof(buf)," \"%s\": [\n", LEGEND_TAG); @@ -714,13 +708,13 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t snprintf(buf,sizeof(buf)," \n", LEGEND_TAG); } addToBuffer(buffer,buf,0); - + /* add graphs and prints */ if (rrd_xport_format_addprints(json,buffer,im)) {return -1;} /* if we have got a trailing , then kill it */ if (buffer->data) { - if (buffer->data[buffer->len-2]==',') { + if (buffer->data[buffer->len-2]==',') { buffer->data[buffer->len-2]=buffer->data[buffer->len-1]; buffer->len--; } @@ -734,7 +728,7 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t } addToBuffer(buffer,buf,0); - + /* start data */ if (json){ snprintf(buf,sizeof(buf)," \"%s\": [\n",DATA_TAG); @@ -774,7 +768,7 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t newval = *ptr; if (json){ if (isnan(newval) || isinf(newval)){ - addToBuffer(buffer,"null",0); + addToBuffer(buffer,"null",0); } else { rrd_snprintf(buf,sizeof(buf),"%0.10e",newval); addToBuffer(buffer,buf,0); @@ -800,11 +794,11 @@ int rrd_xport_format_xmljson(int flags,stringbuffer_t *buffer,image_desc_t *im,t addToBuffer(buffer,buf,0); } ptr++; - } + } if (json){ addToBuffer(buffer,(ti < end-(time_t)step ? " ],\n" : " ]\n"),0); } - else { + else { snprintf(buf,sizeof(buf),"\n", DATA_ROW_TAG); addToBuffer(buffer,buf,0); } @@ -855,9 +849,9 @@ void escapeJSON(char* txt,size_t len) { int rrd_xport_format_addprints(int flags,stringbuffer_t *buffer,image_desc_t *im) { /* initialize buffer */ - stringbuffer_t prints={1024,0,NULL,NULL}; - stringbuffer_t rules={1024,0,NULL,NULL}; - stringbuffer_t gprints={4096,0,NULL,NULL}; + stringbuffer_t prints={1024,0,NULL,NULL}; + stringbuffer_t rules={1024,0,NULL,NULL}; + stringbuffer_t gprints={4096,0,NULL,NULL}; char buf[256]; char dbuf[1024]; char* val; @@ -892,7 +886,7 @@ int rrd_xport_format_addprints(int flags,stringbuffer_t *buffer,image_desc_t *im /* decide to which buffer we print to*/ stringbuffer_t *usebuffer=&gprints; char* usetag="gprint"; - if (im->gdes[i].gf==GF_PRINT) { + if (im->gdes[i].gf==GF_PRINT) { usebuffer=&prints; usetag="print"; } @@ -914,7 +908,7 @@ int rrd_xport_format_addprints(int flags,stringbuffer_t *buffer,image_desc_t *im validsteps++; continue; } - + switch (im->gdes[i].cf) { case CF_HWPREDICT: case CF_MHWPREDICT: @@ -1044,7 +1038,7 @@ int rrd_xport_format_addprints(int flags,stringbuffer_t *buffer,image_desc_t *im } addToBuffer(&rules,buf,0); break; - default: + default: break; } } -- 2.47.2