From: Jeff King <peff@peff.net>
Date: Tue, 19 Mar 2024 21:04:28 GMT
Subject: Re: [PATCH v3 2/2] config: learn the "hostname:" includeIf condition
Message-ID: <20240319210428.GC1159535@coredump.intra.peff.net>
In-Reply-To: <20240319183722.211300-3-ignacio@iencinas.com>

On Tue, Mar 19, 2024 at 07:37:22PM +0100, Ignacio Encinas wrote:

> +`hostname`::
> +	The data that follows the keyword `hostname:` is taken to be a
> +	pattern with standard globbing wildcards. If the current
> +	hostname (output of gethostname(2)) matches the
> +	pattern, the include condition is met.

I was going to comment further here, but I see Eric already replied with
everything I was going to say. ;)

One small comment on the patch...

> +static int include_by_hostname(const char *cond, size_t cond_len)
> +{
> +	int ret;
> +	char my_host[HOST_NAME_MAX + 1];
> +	struct strbuf pattern = STRBUF_INIT;
> +
> +	if (xgethostname(my_host, sizeof(my_host)))
> +		return 0;
> +
> +	strbuf_add(&pattern, cond, cond_len);
> +	ret = !wildmatch(pattern.buf, my_host, 0);
> +	strbuf_release(&pattern);
> +	return ret;
> +}

This is absolutely a nit, but I think using xmemdupz() like:

  char *pattern;
  ...

  pattern = xmemdupz(cond, cond_len);
  ...
  free(pattern);

expresses the intent more directly (it's also a little more efficient,
but that's probably not measurable).

-Peff