Applied (totally agree about apr_getopt_option_t, that's a better name).
Given the interface we've decided on, the copying of argv seems
inevitable (I suppose we could defer copying until we know we'll need
to permute... but that's getting silly :-) ).
-K
> Two changes here:
>
> * apr_longopt_t gets renamed to apr_getopt_option_t.
>
> * argv is accepted as a "char *const *argv", and we make a
> copy in order to permute it. This is a compromise. The
> parameter to be compatible with the argv value passed to
> main, since that is the primary purpose of the function.
> But as has been pointed out, people might want to use the
> function with arrays other than argv, and we shouldn't touch
> the caller's data.
>
> Index: include/apr_getopt.h
> ===================================================================
> RCS file: /home/cvspublic/apr/include/apr_getopt.h,v
> retrieving revision 1.23
> diff -u -r1.23 apr_getopt.h
> --- include/apr_getopt.h 2000/11/25 02:19:40 1.23
> +++ include/apr_getopt.h 2000/11/25 03:25:13
> @@ -77,7 +77,7 @@
> /** count of arguments */
> int argc;
> /** array of pointers to arguments */
> - char **argv;
> + const char **argv;
> /** argument associated with option */
> char const* place;
> /** set to nonzero to support interleaving */
> @@ -87,9 +87,9 @@
> int skip_end;
> };
>
> -typedef struct apr_longopt_t apr_longopt_t;
> +typedef struct apr_getopt_option_t apr_getopt_option_t;
>
> -struct apr_longopt_t {
> +struct apr_getopt_option_t {
> /** long option name, or NULL if option has no long name */
> const char *name;
> /** option letter, or a value greater than 255 if option has no letter */
> @@ -108,7 +108,7 @@
> * @deffunc apr_status_t apr_initopt(apr_getopt_t **os, apr_pool_t *cont,int argc, char *const *argv)
> */
> APR_DECLARE(apr_status_t) apr_initopt(apr_getopt_t **os, apr_pool_t *cont,
> - int argc, char **argv);
> + int argc, char *const *argv);
>
> /**
> * Parse the options initialized by apr_initopt().
> @@ -135,12 +135,12 @@
> * options beginning with "--" in addition to single-character
> * options beginning with "-".
> * @param os The apr_getopt_t structure created by apr_initopt()
> - * @param opts A pointer to a list of apr_longopt_t structures, which can
> - * be initialized with { "name", optch, has_args }. has_args
> + * @param opts A pointer to a list of apr_getopt_option_t structures, which
> + * can be initialized with { "name", optch, has_args }. has_args
> * is nonzero if the option requires an argument. A structure
> * with an optch value of 0 terminates the list.
> - * @param optch Receives the value of "optch" from the apr_longopt_t structure
> - * corresponding to the next option matched.
> + * @param optch Receives the value of "optch" from the apr_getopt_option_t
> + * structure corresponding to the next option matched.
> * @param optarg Receives the argument following the option, if any.
> * @tip There are four potential status values on exit. They are:
> * <PRE>
> @@ -154,9 +154,9 @@
> * os->err is set to 0. If os->interleave is set to nonzero, options can come
> * after arguments, and os->argv will be permuted to leave non-option arguments
> * at the end.
> - * @deffunc apr_status_t apr_getopt_long(apr_getopt_t *os, const apr_longopt_t *opts, int *optch, const char **optarg)
> + * @deffunc apr_status_t apr_getopt_long(apr_getopt_t *os, const apr_getopt_option_t *opts, int *optch, const char **optarg)
> */
> APR_DECLARE(apr_status_t) apr_getopt_long(apr_getopt_t *os,
> - const apr_longopt_t *opts,
> + const apr_getopt_option_t *opts,
> int *optch, const char **optarg);
> #endif /* ! APR_GETOPT_H */
> Index: misc/unix/getopt.c
> ===================================================================
> RCS file: /home/cvspublic/apr/misc/unix/getopt.c,v
> retrieving revision 1.25
> diff -u -r1.25 getopt.c
> --- misc/unix/getopt.c 2000/11/25 02:19:43 1.25
> +++ misc/unix/getopt.c 2000/11/25 03:25:18
> @@ -49,14 +49,18 @@
> }
>
> APR_DECLARE(apr_status_t) apr_initopt(apr_getopt_t **os, apr_pool_t *cont,
> - int argc, char **argv)
> + int argc, char *const *argv)
> {
> + int i;
> +
> *os = apr_palloc(cont, sizeof(apr_getopt_t));
> (*os)->cont = cont;
> (*os)->err = 1;
> (*os)->place = EMSG;
> (*os)->argc = argc;
> - (*os)->argv = argv;
> + (*os)->argv = apr_palloc(cont, argc * sizeof(const char *));
> + for (i = 0; i < argc; i++)
> + (*os)->argv[i] = argv[i];
> (*os)->interleave = 0;
> (*os)->ind = 1;
> (*os)->skip_start = 1;
> @@ -135,9 +139,9 @@
> }
>
> /* Reverse the sequence argv[start..start+len-1]. */
> -static void reverse(char **argv, int start, int len)
> +static void reverse(const char **argv, int start, int len)
> {
> - char *temp;
> + const char *temp;
>
> for (; len >= 2; start++, len -= 2) {
> temp = argv[start];
> @@ -193,7 +197,7 @@
> }
>
> APR_DECLARE(apr_status_t) apr_getopt_long(apr_getopt_t *os,
> - const apr_longopt_t *opts,
> + const apr_getopt_option_t *opts,
> int *optch, const char **optarg)
> {
> const char *p;
------- End of forwarded message -------
Received on Sat Oct 21 14:36:15 2006