The error messages of getopt are tricky to localise properly, since
they are served as fprintf-like function invocations only. Since there
is only very few distinct messages, the best solution is to introduce
some sort of codes that the application can do whatever it wants with.
Here is a patch that does exactly that. It is entirely source and, by
virtue of a slightly sleazy hack, binary compatible. That hack can be
eliminated if we get to break binary compatibility; source
compatibility would still be retained. I'd suggest using the hack in
1.x and doing it cleanly (by adding a apr_getopt_t member) in 2.x.
The getopt code is ancient and it shows: nobody would have exposed a
struct like that in an API design today, but it's what we have to work
with.
Please review, and tell me if I should supply a commit message,
CHANGES addition and so on. The patch is against trunk.
Index: include/apr_getopt.h
===================================================================
--- include/apr_getopt.h (revision 1548533)
+++ include/apr_getopt.h (arbetskopia)
@@ -92,6 +92,22 @@ struct apr_getopt_option_t {
};
/**
+ * Error codes returned from apr_getopt_errcode().
+ *
+ * The associated values returned from apr_getopt_errparam() are given
+ * below for each error code.
+ */
+typedef enum {
+ APR_GETOPT_INVALID_OPTION, /**< Invalid option; error parameter
+ is the option. */
+ APR_GETOPT_MISSING_ARGUMENT, /**< Missing argument; error parameter is
+ the option whose argument is
+ missing. */
+ APR_GETOPT_INVALID_ARGUMENT /**< Invalid or malformed argument; error
+ parameter is the invalid argument. */
+} apr_getopt_error_e;
+
+/**
* Initialize the arguments for parsing by apr_getopt().
* @param os The options structure created for apr_getopt()
* @param cont The pool to operate on
@@ -151,6 +167,30 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
const apr_getopt_option_t *opts,
int *option_ch,
const char **option_arg);
+
+/**
+ * Retrieve a code describing the current getopt error. This function
+ * may only be called from the getopt error function, @c errfn
+ * in @c apr_getopt_t.
+ * @param os The apr_getopt_t structure created by apr_getopt_init().
+ * @return The error code.
+ * Error codes may have an associated string parameter;
+ * use apr_getopt_errparam() to retrieve it.
+ */
+APR_DECLARE(apr_getopt_error_e) apr_getopt_errcode(apr_getopt_t *os);
+
+/**
+ * Retrieve a string parameter pertaining to the current getopt error.
+ * This function may only be called from the getopt error function, @c errfn
+ * in @c apr_getopt_t.
+ * @param os The apr_getopt_t structure created by apr_getopt_init().
+ * @return The error parameter string. It is only valid during the error
+ * function call.
+ * The meaning of the returned string depends on the current getopt error code;
+ * use apr_getopt_errcode() to retrieve it.
+ */
+APR_DECLARE(const char *) apr_getopt_errparam(apr_getopt_t *os);
+
/** @} */
#ifdef __cplusplus
Index: misc/unix/getopt.c
===================================================================
--- misc/unix/getopt.c (revision 1548533)
+++ misc/unix/getopt.c (arbetskopia)
@@ -68,6 +68,42 @@ APR_DECLARE(apr_status_t) apr_getopt_init(apr_geto
return APR_SUCCESS;
}
+struct error_info_t {
+ apr_getopt_error_e code;
+ const char *param;
+};
+
+APR_DECLARE(apr_getopt_error_e) apr_getopt_errcode(apr_getopt_t *os)
+{
+ struct error_info_t *e = os->errarg;
+ return e->code;
+}
+
+APR_DECLARE(const char *) apr_getopt_errparam(apr_getopt_t *os)
+{
+ struct error_info_t *e = os->errarg;
+ return e->param;
+}
+
+/* Print out an error by calling the user-defined error callback, if any. */
+static void perr(apr_getopt_t *os, apr_getopt_error_e err,
+ const char *msg, const char *par)
+{
+ if (os->errfn) {
+ /* Hack: we temporarily re-purpose the errarg member for
+ holding a reference to the error code and string parameter
+ during the call to the error function. */
+ void *errarg = os->errarg;
+ struct error_info_t e;
+ e.code = err;
+ e.param = par;
+ os->errarg = &e;
+ (os->errfn)(errarg, "%s: %s: %s\n",
+ apr_filepath_name_get(*os->argv), msg, par);
+ os->errarg = errarg;
+ }
+}
+
APR_DECLARE(apr_status_t) apr_getopt(apr_getopt_t *os, const char *opts,
char *optch, const char **optarg)
{
@@ -99,9 +135,11 @@ APR_DECLARE(apr_status_t) apr_getopt(apr_getopt_t
}
if (!*os->place)
++os->ind;
- if (os->errfn && *opts != ':') {
- (os->errfn)(os->errarg, "%s: illegal option -- %c\n",
- apr_filepath_name_get(*os->argv), os->opt);
+ if (*opts != ':') {
+ char par[2] = "x";
+ par[0] = os->opt;
+ perr(os, APR_GETOPT_INVALID_OPTION,
+ "illegal option", par);
}
*optch = os->opt;
return (APR_BADCH);
@@ -115,16 +153,15 @@ APR_DECLARE(apr_status_t) apr_getopt(apr_getopt_t
if (*os->place) /* no white space */
*optarg = os->place;
else if (os->argc <= ++os->ind) { /* no arg */
+ char par[2] = "x";
os->place = EMSG;
if (*opts == ':') {
*optch = os->opt;
return (APR_BADARG);
}
- if (os->errfn) {
- (os->errfn)(os->errarg,
- "%s: option requires an argument -- %c\n",
- apr_filepath_name_get(*os->argv), os->opt);
- }
+ par[0] = os->opt;
+ perr(os, APR_GETOPT_MISSING_ARGUMENT,
+ "option requires an argument", par);
*optch = os->opt;
return (APR_BADCH);
}
@@ -177,26 +214,6 @@ static void permute(apr_getopt_t *os)
os->skip_end += len2;
}
-/* Helper function to print out an error involving a long option */
-static apr_status_t serr(apr_getopt_t *os, const char *err, const char *str,
- apr_status_t status)
-{
- if (os->errfn)
- (os->errfn)(os->errarg, "%s: %s: %s\n",
- apr_filepath_name_get(*os->argv), err, str);
- return status;
-}
-
-/* Helper function to print out an error involving a short option */
-static apr_status_t cerr(apr_getopt_t *os, const char *err, int ch,
- apr_status_t status)
-{
- if (os->errfn)
- (os->errfn)(os->errarg, "%s: %s: %c\n",
- apr_filepath_name_get(*os->argv), err, ch);
- return status;
-}
-
APR_DECLARE(apr_status_t) apr_getopt_long(apr_getopt_t *os,
const apr_getopt_option_t *opts,
int *optch, const char **optarg)
@@ -236,8 +253,11 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
p++;
for (i = 0; ; i++) {
- if (opts[i].optch == 0) /* No match */
- return serr(os, "invalid option", p - 2, APR_BADCH);
+ if (opts[i].optch == 0) { /* No match */
+ perr(os, APR_GETOPT_INVALID_OPTION,
+ "invalid option", p - 2);
+ return APR_BADCH;
+ }
if (opts[i].name) {
len = strlen(opts[i].name);
@@ -252,15 +272,20 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
if (p[len] == '=') /* Argument inline */
*optarg = p + len + 1;
else {
- if (os->ind >= os->argc) /* Argument missing */
- return serr(os, "missing argument", p - 2, APR_BADARG);
- else /* Argument in next arg */
+ if (os->ind >= os->argc) { /* Argument missing */
+ perr(os, APR_GETOPT_MISSING_ARGUMENT,
+ "missing argument", p - 2);
+ return APR_BADARG;
+ } else /* Argument in next arg */
*optarg = os->argv[os->ind++];
}
} else {
*optarg = NULL;
- if (p[len] == '=')
- return serr(os, "erroneous argument", p - 2, APR_BADARG);
+ if (p[len] == '=') {
+ perr(os, APR_GETOPT_INVALID_ARGUMENT,
+ "erroneous argument", p - 2);
+ return APR_BADARG;
+ }
}
permute(os);
return APR_SUCCESS;
@@ -271,8 +296,11 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
return APR_EOF;
}
else
- if (*p == '\0') /* Bare "-" is illegal */
- return serr(os, "invalid option", p, APR_BADCH);
+ if (*p == '\0') { /* Bare "-" is illegal */
+ perr(os, APR_GETOPT_INVALID_OPTION,
+ "invalid option", p - 1);
+ return APR_BADCH;
+ }
}
}
@@ -281,8 +309,13 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
* Look for it in the caller's table.
*/
for (i = 0; ; i++) {
- if (opts[i].optch == 0) /* No match */
- return cerr(os, "invalid option character", *p, APR_BADCH);
+ if (opts[i].optch == 0) { /* No match */
+ char arg[2] = "x";
+ arg[0] = *p;
+ perr(os, APR_GETOPT_INVALID_OPTION,
+ "invalid option", arg);
+ return APR_BADCH;
+ }
if (*p == opts[i].optch)
break;
@@ -293,9 +326,13 @@ APR_DECLARE(apr_status_t) apr_getopt_long(apr_geto
if (*p != '\0') /* Argument inline */
*optarg = p;
else {
- if (os->ind >= os->argc) /* Argument missing */
- return cerr(os, "missing argument", *optch, APR_BADARG);
- else /* Argument in next arg */
+ if (os->ind >= os->argc) { /* Argument missing */
+ char arg[2] = "x";
+ arg[0] = *optch;
+ perr(os, APR_GETOPT_MISSING_ARGUMENT,
+ "missing argument", arg);
+ return APR_BADARG;
+ } else /* Argument in next arg */
*optarg = os->argv[os->ind++];
}
os->place = EMSG;
Index: test/testargs.c
===================================================================
--- test/testargs.c (revision 1548533)
+++ test/testargs.c (arbetskopia)
@@ -30,13 +30,25 @@ static void format_arg(char *str, char option, con
}
}
-static void unknown_arg(void *str, const char *err, ...)
+typedef struct {
+ apr_getopt_t *opt;
+ char *formatted;
+ char *param;
+ int errcode; /* Actually an apr_getopt_error_e, but declared
+ int so that it can contain an OOB value. */
+} error_data_t;
+
+static void unknown_arg(void *arg, const char *err, ...)
{
+ error_data_t *ed = arg;
va_list va;
va_start(va, err);
- apr_vsnprintf(str, 8196, err, va);
+ apr_vsnprintf(ed->formatted, 8196, err, va);
va_end(va);
+
+ ed->errcode = apr_getopt_errcode(ed->opt);
+ strncpy(ed->param, apr_getopt_errparam(ed->opt), 128);
}
static void no_options_found(abts_case *tc, void *data)
@@ -78,13 +90,21 @@ static void no_options(abts_case *tc, void *data)
char ch;
const char *opt_arg;
char str[8196];
+ char par[128];
+ error_data_t ed;
str[0] = '\0';
+ par[0] = '\0';
rv = apr_getopt_init(&opt, p, largc, largv);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ed.opt = opt;
+ ed.formatted = str;
+ ed.param = par;
+ ed.errcode = -1;
+
opt->errfn = unknown_arg;
- opt->errarg = str;
+ opt->errarg = &ed;
while (apr_getopt(opt, "efgh", &ch, &opt_arg) == APR_SUCCESS) {
switch (ch) {
@@ -98,7 +118,9 @@ static void no_options(abts_case *tc, void *data)
break;
}
}
- ABTS_STR_EQUAL(tc, "testprog: illegal option -- a\n", str);
+ ABTS_STR_EQUAL(tc, "testprog: illegal option: a\n", str);
+ ABTS_STR_EQUAL(tc, "a", par);
+ ABTS_INT_EQUAL(tc, APR_GETOPT_INVALID_OPTION, ed.errcode);
}
static void required_option(abts_case *tc, void *data)
@@ -110,13 +132,19 @@ static void required_option(abts_case *tc, void *d
char ch;
const char *opt_arg;
char str[8196];
+ error_data_t ed;
str[0] = '\0';
rv = apr_getopt_init(&opt, p, largc, largv);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ed.opt = opt;
+ ed.formatted = str;
+ ed.param = NULL;
+ ed.errcode = -1;
+
opt->errfn = unknown_arg;
- opt->errarg = str;
+ opt->errarg = &ed;
while (apr_getopt(opt, "a:", &ch, &opt_arg) == APR_SUCCESS) {
switch (ch) {
@@ -128,6 +156,7 @@ static void required_option(abts_case *tc, void *d
}
}
ABTS_STR_EQUAL(tc, "option: a with foo\n", str);
+ ABTS_INT_EQUAL(tc, -1, ed.errcode);
}
static void required_option_notgiven(abts_case *tc, void *data)
@@ -139,13 +168,21 @@ static void required_option_notgiven(abts_case *tc
char ch;
const char *opt_arg;
char str[8196];
+ char par[128];
+ error_data_t ed;
str[0] = '\0';
+ par[0] = '\0';
rv = apr_getopt_init(&opt, p, largc, largv);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ed.opt = opt;
+ ed.formatted = str;
+ ed.param = par;
+ ed.errcode = -1;
+
opt->errfn = unknown_arg;
- opt->errarg = str;
+ opt->errarg = &ed;
while (apr_getopt(opt, "a:", &ch, &opt_arg) == APR_SUCCESS) {
switch (ch) {
@@ -156,7 +193,9 @@ static void required_option_notgiven(abts_case *tc
break;
}
}
- ABTS_STR_EQUAL(tc, "testprog: option requires an argument -- a\n", str);
+ ABTS_STR_EQUAL(tc, "testprog: option requires an argument: a\n", str);
+ ABTS_STR_EQUAL(tc, "a", par);
+ ABTS_INT_EQUAL(tc, APR_GETOPT_MISSING_ARGUMENT, ed.errcode);
}
static void optional_option(abts_case *tc, void *data)
@@ -168,13 +207,19 @@ static void optional_option(abts_case *tc, void *d
char ch;
const char *opt_arg;
char str[8196];
+ error_data_t ed;
str[0] = '\0';
rv = apr_getopt_init(&opt, p, largc, largv);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ed.opt = opt;
+ ed.formatted = str;
+ ed.param = NULL;
+ ed.errcode = -1;
+
opt->errfn = unknown_arg;
- opt->errarg = str;
+ opt->errarg = &ed;
while (apr_getopt(opt, "a::", &ch, &opt_arg) == APR_SUCCESS) {
switch (ch) {
@@ -186,6 +231,7 @@ static void optional_option(abts_case *tc, void *d
}
}
ABTS_STR_EQUAL(tc, "option: a with foo\n", str);
+ ABTS_INT_EQUAL(tc, -1, ed.errcode);
}
static void optional_option_notgiven(abts_case *tc, void *data)
@@ -197,13 +243,21 @@ static void optional_option_notgiven(abts_case *tc
char ch;
const char *opt_arg;
char str[8196];
+ char par[128];
+ error_data_t ed;
str[0] = '\0';
+ par[0] = '\0';
rv = apr_getopt_init(&opt, p, largc, largv);
ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+ ed.opt = opt;
+ ed.formatted = str;
+ ed.param = par;
+ ed.errcode = -1;
+
opt->errfn = unknown_arg;
- opt->errarg = str;
+ opt->errarg = &ed;
while (apr_getopt(opt, "a::", &ch, &opt_arg) == APR_SUCCESS) {
switch (ch) {
@@ -218,7 +272,9 @@ static void optional_option_notgiven(abts_case *tc
/* Our version of getopt doesn't allow for optional arguments. */
ABTS_STR_EQUAL(tc, "option: a\n", str);
#endif
- ABTS_STR_EQUAL(tc, "testprog: option requires an argument -- a\n", str);
+ ABTS_STR_EQUAL(tc, "testprog: option requires an argument: a\n", str);
+ ABTS_STR_EQUAL(tc, "a", par);
+ ABTS_INT_EQUAL(tc, APR_GETOPT_MISSING_ARGUMENT, ed.errcode);
}
abts_suite *testgetopt(abts_suite *suite)
