Re: [PATCH] xgethostname: handle long hostnames
- Date: Fri, 14 Apr 2017 00:32:03 +0200
- From: René Scharfe <l.s.r@xxxxxx>
- Subject: Re: [PATCH] xgethostname: handle long hostnames
Am 13.04.2017 um 21:23 schrieb David Turner:
> If the full hostname doesn't fit in the buffer supplied to
> gethostname, POSIX does not specify whether the buffer will be
> null-terminated, so to be safe, we should do it ourselves. Introduce
> new function, xgethostname, which ensures that there is always a \0
> at the end of the buffer.
> Signed-off-by: David Turner <dturner@xxxxxxxxxxxx>
> diff --git a/wrapper.c b/wrapper.c
> index 0542fc7582..d837417709 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -655,3 +655,16 @@ void sleep_millisec(int millisec)
> poll(NULL, 0, millisec);
> +int xgethostname(char *buf, size_t len)
> + /*
> + * If the full hostname doesn't fit in buf, POSIX does not
> + * specify whether the buffer will be null-terminated, so to
> + * be safe, do it ourselves.
> + */
> + int ret = gethostname(buf, len);
> + if (!ret)
> + buf[len - 1] = 0;
> + return ret;
Silent truncation is not ideal, no matter if it's done by the wrapper or
the original function. It would be better to use a properly sized
POSIX requires hostnames to have a maximum length of HOST_NAME_MAX. So
how about just adding an assert to make sure len is big enough? Or
evaluate the condition at compile time with BUILD_ASSERT_OR_ZERO?
Downside: Not all platforms define HOST_NAME_MAX. daemon.c uses 256 as
a fallback. On Windows a buffer size of 256 is documented to suffice
in all cases . The Linux manpage  mentions a hostname length
limit of 255 (plus NUL) as well, even though HOST_NAME_MAX is 64 there.
Another possibility: Die (or at least warn) if the buffer doesn't
contain a NUL byte after calling gethostname(). That only works for
platforms that don't NUL-terminate on truncation, though, so silent
truncation would still go unnoticed.
Anyway, the buffer in builtin/gc.c with its 128 bytes seems to be too
short; the others are at least 256 bytes long. Replacing the magic
buffer size number with HOST_NAME_MAX + 1 might be a good idea (after
moving the fallback definition to git-compat-util.h).