[m-rev.] for review: MERCURY_OPTIONS-progname

[Zoltan Somogyi]

On 21-May-2008, Paul Bone <pbone at csse.unimelb.edu.au> wrote:
> > +    gen_env_options = getenv(MERCURY_OPTIONS);
> > +    if (gen_env_options == NULL) {
> > +        gen_env_options = (char *) "";
> > +    }
> 
> Why use a cast here?
> 
> I beleive that "" is normaly const char *.  Can gen_env_options be made
> const char *?

It has to be treated the same way as main's argv, since it is processed
by the same function. Argv is char *, since it predates the invention of const.

> > +            if (gen_env_options[0] != '\0') {
> > +                sprintf(where_buf + where_buf_next,
> > +                    "%sthe %s environment variable",
> > +                    where_buf_next == 0 ? "" : " and/or ",
> > +                    MERCURY_OPTIONS);
> > +                where_buf_next = strlen(where_buf);
> 
> This could crash when supplied with a large-enough invalid command line
> option.

It can't, because MERCURY_OPTIONS is the name of the environment variable,
not its value. There is even a 256-character limit on the name of the
executable.

> snprintf will not overflow the buffer.

That is a good idea. Done.

Zoltan.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------