cpplib: command line argument parsing

[Zack Weinberg]

On Thu, Feb 17, 2000 at 10:36:50PM +0900, Neil Booth wrote:
> This is the current state of my patch to tighten up cpplib's command
> line handling.

It's really getting there!  I do keep nitpicking, but the basic idea
is right on the money.

> o I'm not sure if the internationalisation stuff is quite right -
> could you please confirm if it's what you had in mind, Zack?

More or less.  I don't like the magic numbers.

> o The -W options are strcmp-ed one by one; this may be worth improving
> with a binary search.  Please advise.

It is acceptable for the moment, as it's no worse than we have now.
But please move -Wall to the top and -Wtraditional to second place;
these are the most commonly used.

We can revisit this once the basic patch is in.
    
> o -ifoutput is still accepted

Since it's still accepted in the main tree, this is fine.

I have a couple more kibitzes:

> 	* cppinit.c (opt_comp) New function.  Called by sort_options
> 	as the comparison routine.
> 	(sort_options) New function.  Sorts the array of command line
> 	options at run time for non-ASCII host environments.
> 	(parse_option) New function.  Parses a single command line
> 	option, possibly taking an argument.

You don't need to say what new functions do in the changelog.  That
goes in the comment above the function, in the code.

> 	(cpp_handle_option) Rewritten to use parse_option for binary
> 	searching of options.
> 	(cpp_handle_options) Sorts the array of command line options
> 	during its first invocation.

Imperative voice:

	(cpp_handle_option): On first invocation, sort the array of
	command line options.

> 	(print_help) Updated run-time help.

This suffices:

	(print_help) Update.


> 
> Index: cppinit.c
> ===================================================================
> RCS file: /cvs/gcc/egcs/gcc/cppinit.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 cppinit.c
> --- cppinit.c	2000/02/15 16:36:32	1.41
> +++ cppinit.c	2000/02/17 13:18:26
> @@ -201,6 +201,13 @@ static void initialize_dependency_output
>  static void new_pending_define		PARAMS ((struct cpp_options *,
>  						 const char *));
>  
> +#ifdef HOST_EBCDIC
> +static int opt_comp                     PARAMS ((const void*, const void*));
> +#endif
> +static void sort_options                PARAMS ((void));

Just call qsort directly, and see below.

...
> +struct cl_option
> +{
> +  const char* opt_text;
> +  int arg_index;	/* -1 = takes no argument */
> +  size_t opt_len;
> +  enum opt_code opt_code;
> +};

On 64-bit machines, this structure will have unnecessary padding.  Put
opt_len before arg_index.


> +
> +/* Run-time sorted array of pointers to command-line options below */
> +static const struct cl_option* cl_options[N_OPTS];

Don't introduce this extra layer of indirection.  qsort is quite
capable of sorting the cl_opt_array.

...
> +/* These lists must be ASCII sorted. Make enum order above match this. */
> +#define DEF_OPT(text, msg_idx, code) {text, msg_idx, sizeof(text) - 1, code}

/* On EBCDIC platforms, this list must be sorted at runtime.  */
#ifdef HOST_EBCDIC
static struct cl_option cl_opt_array =
#else
> +static const struct cl_option cl_opt_array[] =
#endif
> +{
> +  DEF_OPT("",                      -1, OPT_stdin_stdout),

...
> +  DEF_OPT("W",                      0, OPT_W), /* NB: Immed arg only, and not reqd */

Word wrap to 78 columns, or shorten the comment.

...
>    int i;
>    int strings_processed;

#ifdef HOST_EBCDIC
> +  static int opts_sorted = 0;
> +
> +  if (!opts_sorted)
> +    {
> +      opts_sorted = 1;
> +      sort_options();

Even if the function has no arguments, you must put a space between
the name and the open paren.  (No, I don't like it either.)

> +    }
#endif