Re: [PATCH 2/2] bisect: rewrite `check_term_format` shell function in C

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 4, 2016, 11:13 UTC
Message-ID: <alpine.DEB.2.20.1605041309430.9313@virtualbox>
In-Reply-To: <CAFZEwPNKug1pvGC1fTvZzVPBGKy71fw6S3qcx_fx98nYZasR3w@mail.gmail.com>

Hi Pranit,

On Wed, 4 May 2016, Pranit Bauva wrote:

Show 17 quoted lines

> On Wed, May 4, 2016 at 12:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Wed, May 4, 2016 at 1:07 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> >
> >> +static int one_of(const char *term, ...)
> >> +{
> >> +       va_list matches;
> >> +       const char *match;
> >> +
> >> +       va_start(matches, term);
> >> +       while ((match = va_arg(matches, const char *)) != NULL)
> >> +               if (!strcmp(term, match))
> >> +                       return 1;
> >
> > Is it wise to return here without invoking va_end()?
> 
> I guess since it already checks for NULL, invoking va_end() will make
> it redundant[3].

Actually, this is my fault (I suggested that form of the one_of() function). The man page for va_end() clearly states that every call to va_start() needs to be paired with a corresponding va_end(), so it is incorrect to return without a call to va_end().

Maybe something like instead?

	static int one_of(const char *term, ...)
	{
		int res = 0;
		va_list matches;
		const char *match;

		va_start(matches, term);
		while (!res && (match = va_arg(matches, const char *)))
			res = !strcmp(term, match);
		va_end(matches);

		return res;
	}
Ciao,
Dscho