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

From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Date: Feb 25, 2022, 08:08 UTC
Message-ID: <20220225080811.8097-1-chakrabortyabhradeep79@gmail.com>
In-Reply-To: <xmqqh78nh3sf.fsf@gitster.g>

Junio C Hamano wrote:

Show 10 quoted lines

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

Oh, sorry for that. I was in kind of a hurry ( today was my semester exam), so I didn't look at the style guide. Will fix it.

Show 12 quoted lines

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

hmm, I wanted a formatting function to format (i.e. pass the `opt->help` dynamically) the output string. The existing use of `optbug()` that you specified has no `%s` formatter; it is a plain string. That's why I used `xstrfmt()`. Moreover, it was in Ævar's suggestion[1] -

> +		if (opts->help && ends_with(opts->help, "."))
> +			err |= optbug(opts, xstrfmt("argh should not end with a dot: %s", opts->help));

But I think, you're right. There is some memory leakage here. Should I go with plain strings then? (i.e. "help should not end with a dot" instead of `xstrfmt("help should not end with a dot: %s", opts->help)`)

Thanks :)

[1] https://lore.kernel.org/git/220221.86tucsb4oy.gmgdl@evledraar.gmail.com/