On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
quoted text
> Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > [...]
>=20
> "const struct option *opts"?
>=20
> 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.
>=20
> And you want make opts const.

  Ok.

quoted text
> 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.

  Yes this has been spotted and fixed already.

quoted text
>=20
> > +{
> > +	struct strbuf sb;
> > +
> > +	strbuf_init(&sb, 4096);
> > +	do {
> > +		strbuf_addstr(&sb, *usagestr++);
> > +		strbuf_addch(&sb, '\n');
> > +	} while (*usagestr);
>=20
> 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).

  Yes this is intentional, there should be at least on string in the
usagestr array.


quoted text
> > +     strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
> > +		    opts->help);
> ....
> > +	usage(sb.buf);
>=20
> 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 :)

  the reason is that usage() is a wrapper around a callback, and I
suppose it's used by some GUI's or anything like that.

  FWIW you can rework the .c like this:

  pos =3D 0; /* and not pos =3D sb.len */

  replace the strbuf_add* by the equivalents:
  pos +=3D printf("....");

  and tada, you're done.


  Note that in the most recent version, I also deal with a
OPTION_CALLBACK that passes the value to a callback.

Cheers,
--=20
=C2=B7O=C2=B7  Pierre Habouzit
=C2=B7=C2=B7O                                                madcoder@debia=
n.org
OOO                                                http://www.madism.org