Subject: Re: [PATCH v4 2/2] parse-options.c: add style checks for usage-strings

From: Junio C Hamano <[email-masked]>
Date: Thu, 24 Feb 2022 22:13:36 -0800

"Abhradeep Chakraborty via GitGitGadget" <[email-masked]>
writes:

> +
> +		// OPTION_GROUP should be ignored
> +		// if the first two characters of the help string are uppercase, then assume it is an
> +		// acronym (i.e. "GPG") or special name (i.e. "HEAD"), thus allowed.
> +		// else assume the usage string is violating the style convention and throw error.

Style.

	/*
	 * This is how our multi-line comments
         * look like; with slash-asterisk that opens
         * and asterisk-slash that closes one on their
         * own lines.
	 */

Also avoid overly long lines.

> +		if (opts->type != OPTION_GROUP && opts->help &&
> +			opts->help[0] && isupper(opts->help[0]) &&
> +			!(opts->help[1] && isupper(opts->help[1])))
> +			err |= optbug(opts, xstrfmt("help should not start with capital letter unless needed: %s", opts->help));
> +		if (opts->help && !ends_with(opts->help, "...") && ends_with(opts->help, "."))
> +			err |= optbug(opts, xstrfmt("help should not end with a dot: %s", opts->help));

These two calls to optbug() use xstrfmt() to grab allocated pieces
of memory and pass it as a parameter to the function, which means
the string is leaked without any chance to be freed.

Do we care?

>  		if (opts->argh &&
>  		    strcspn(opts->argh, " _") != strlen(opts->argh))
>  			err |= optbug(opts, "multi-word argh should use dash to separate words");

The existing use of optbug() we see here does not share such a
problem.