Looks good overall! I've got a few nits, below.
On Fri, Nov 24, 2000 at 06:46:25PM -0500, Greg Hudson wrote:
>...
> - char *const *argv;
> + char **argv;
Um. I don't think we can do this with argv. I'm surprised that it isn't
"const char * const * argv".
Certainly, if we make a copy of the array (of pointers), then we could have
"const char **argv" and that would allow us to permute.
>...
> -typedef struct apr_getopt_long_t apr_getopt_long_t;
> +typedef struct apr_option_t apr_option_t;
>
> -/* structure representing a single longopt */
> -struct apr_getopt_long_t {
> - /** the name of the long argument (sans "--") */
> +struct apr_option_t {
I think the name "apr_option_t" is a bit too generic. The apr_getopt_ prefix
probably ought to remain.
>...
> @@ -106,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 *const *argv);
> + int argc, char **argv);
This should be "const char * const * argv". We can make copies internally,
if we choose. The changing prototype will have an effect upon Apache, but
that isn't a problem... I can easily update those references.
[ there may be some test proggies in apr/test/ that would need changing, too ]
>...
> @@ -57,6 +58,8 @@
> (*os)->place = EMSG;
> (*os)->argc = argc;
> (*os)->argv = argv;
> + (*os)->interleave = 0;
> + (*os)->skip_start = (*os)->skip_end = (*os)->ind;
Can we simplify this and make it:
(*os)->skip_start = 1;
(*os)->skip_end = 1;
By using the other variable, then I need to look back to figure out just
what is going on. (and in this case, I didn't even have the context, so I
needed to bring up the file to see what the above line meant)
>...
> +static void reverse(char **argv, int start, int len)
> +{
> + char *temp;
Constify on the above two lines...
>...
> + else if (os->ind >= os->argc) /* Argument missing */
> + return cerr(os, "option requires an argument", *optch, APR_BADARG);
The error message is different from the similar error for long options.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
