Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
quoted text
> +static int opterror(struct option *opt, const char *reason, int flags)

"const struct option *opt"? You never modify the struct option itself,
only the values under the pointers it contains. Using const here will
allow the compiler to reuse string constants (not that there will be
much of the opportunity, but anyway) in the option arrays.

quoted text
> +static int get_value(struct optparse_t *p, struct option *opt, int flags)

"const struct option *opt"?

quoted text
> +static int parse_short_opt(struct optparse_t *p, struct option *options, int count)

"const struct option *options"?

quoted text
> +int parse_options(int argc, const char **argv,
> +                  struct option *options, int count,
> +				  const char * const usagestr[], int flags)

"const struct option *options"?

quoted text
> +void make_usage(const char * const usagestr[], struct option *opts, int cnt)

"const struct option *opts"?

Why not "const char *const *usagestr"? Especially if you change
"usagestr" (the pointer itself) later. "[]" is sometimes a hint that
the pointer itself should not be changed, being an array.

And you want make opts const.

BTW, it does not "make" usage. It calls the usage() or prints a usage
description. "make" implies it creates the "usage", which according to
the prototype is later nowhere to be found.

quoted text
> +{
> +	struct strbuf sb;
> +
> +	strbuf_init(&sb, 4096);
> +	do {
> +		strbuf_addstr(&sb, *usagestr++);
> +		strbuf_addch(&sb, '\n');
> +	} while (*usagestr);

This will crash for empty usagestr, like  "{ NULL }". Was it
deliberately? (I'd make it deliberately, if I were you. I'd even used
cnt of opts, to force people to document all options).

quoted text
> +     strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
> +		    opts->help);
...
quoted text
> +	usage(sb.buf);

BTW, if you just printed the usage message out (it is about usage of a
program, isn't it?) and called exit() everyone would be just as happy.
And you wouldn't have to include strbuf (it is the only use of it),
less code, too. It'd make simplier to stea^Wcopy your implementation,
which I like :)

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html