Re: [PATCH v3 1/2] use HOST_NAME_MAX to size buffers for gethostname(2)
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Apr 19, 2017, 01:28 UTC
- Message-ID
- <20170419012824.GA28740@aiede.svl.corp.google.com>
- In-Reply-To
- <20170418215743.18406-2-dturner@twosigma.com>
Hi,
David Turner wrote:
Show 6 quoted lines
> From: René Scharfe <l.s.r@web.de> > > POSIX limits the length of host names to HOST_NAME_MAX. Export the > fallback definition from daemon.c and use this constant to make all > buffers used with gethostname(2) big enough for any possible result > and a terminating NUL.
Since some platforms do not define HOST_NAME_MAX and we provide a fallback, this is not actually big enough for any possible result. For example, the Hurd allows arbitrarily long hostnames.
Nevertheless this patch seems like the right thing to do.
Show 11 quoted lines
> Inspired-by: David Turner <dturner@twosigma.com> > Signed-off-by: Rene Scharfe <l.s.r@web.de> > Signed-off-by: David Turner <dturner@twosigma.com> > --- > builtin/gc.c | 10 +++++++--- > builtin/receive-pack.c | 2 +- > daemon.c | 4 ---- > fetch-pack.c | 2 +- > git-compat-util.h | 4 ++++ > ident.c | 2 +- > 6 files changed, 14 insertions(+), 10 deletions(-)
Thanks for picking this up.
[...]
> +++ b/builtin/gc.c
[...]
Show 20 quoted lines
> @@ -257,8 +257,12 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
> fd = hold_lock_file_for_update(&lock, pidfile_path,
> LOCK_DIE_ON_ERROR);
> if (!force) {
> - static char locking_host[128];
> + static char locking_host[HOST_NAME_MAX + 1];
> + static char *scan_fmt;
> int should_exit;
> +
> + if (!scan_fmt)
> + scan_fmt = xstrfmt("%s %%%dc", "%"SCNuMAX, HOST_NAME_MAX);
> fp = fopen(pidfile_path, "r");
> memset(locking_host, 0, sizeof(locking_host));
> should_exit =
> @@ -274,7 +278,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
> * running.
> */
> time(NULL) - st.st_mtime <= 12 * 3600 &&
> - fscanf(fp, "%"SCNuMAX" %127c", &pid, locking_host) == 2 &&
> + fscanf(fp, scan_fmt, &pid, locking_host) == 2 &&I hoped this could be simplified since HOST_NAME_MAX is a numeric literal, using the double-expansion trick:
#define STR_(s) # s #define STR(s) STR_(s)
fscanf(fp, "%" SCNuMAX " %" STR(HOST_NAME_MAX) "c", &pid, locking_host);
Unfortunately, I don't think there's anything stopping a platform from defining
#define HOST_NAME_MAX 0x100
which would break that.
So this run-time calculation appears to be necessary.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.