Am 19.04.2017 um 17:50 schrieb David Turner: >> -----Original Message----- >> From: Junio C Hamano [mailto:gitster@xxxxxxxxx] >> Sent: Tuesday, April 18, 2017 10:51 PM >> To: Jonathan Nieder <jrnieder@xxxxxxxxx> >> Cc: David Turner <David.Turner@xxxxxxxxxxxx>; git@xxxxxxxxxxxxxxx; >> l.s.r@xxxxxx >> Subject: Re: [PATCH v3 2/2] xgethostname: handle long hostnames >> >> Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >> >>> Hi, >>> >>> David Turner wrote: >>> >>>> 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. >>> >>> I think we should detect the error instead of truncating the hostname. >>> That (on top of your patch) would look like the following. >>> >>> Thoughts? >>> Jonathan >>> >>> diff --git i/wrapper.c w/wrapper.c >>> index d837417709..e218bd3bef 100644 >>> --- i/wrapper.c >>> +++ w/wrapper.c >>> @@ -660,11 +660,13 @@ 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. >>> + * guarantee that an error will be returned. Check for ourselves >>> + * to be safe. >>> */ >>> int ret = gethostname(buf, len); >>> - if (!ret) >>> - buf[len - 1] = 0; >>> + if (!ret && !memchr(buf, 0, len)) { >>> + errno = ENAMETOOLONG; >>> + return -1; >>> + } >> >> Hmmmm. "Does not specify if the buffer will be NUL-terminated" >> would mean that it is OK for the platform gethostname() to stuff >> sizeof(buf)-1 first bytes of the hostname in the buffer and then truncate by >> placing '\0' at the end of the buf, and we would not notice truncation with the >> above change on such a platform, no? > > My read of the docs is that not only is that OK, but it is also permitted > for the platform to put sizeof(buf) bytes into the buffer and *not* > put \0 at the end. That sounds crazy, but that's how I read the spec [1] as well. And POSIX also doesn't specify any errors for gethostname. But that makes kinda sense because it *does* specify HOST_NAME_MAX as maximum size. Things get more interesting when this spec meets systems that don't have HOST_NAME_MAX, or error returns, or bugs. > So in order to do a dynamic approach, we would have to allocate some > buffer, then run gethostname, then check if the penultimate element > of the buffer was written to, and if so, allocate a larger buffer. Yucky, > but possible. That's what the gnulib version of xgethostname does [2], among other things. The more I read about gethostname and its weirdness, the more I think we should import an existing, proven version of xgethostname that returns an allocated buffer. That way we wouldn't have to worry about truncation or missing NULs or buffer sizes anymore. What do you think? I found the one from gnulib and from Neal Walfield [3] mentioned in the Hurd docs; are there more? René [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/gethostname.html [2] http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/xgethostname.c;hb=0632e115747ff96e93330c88f536d7354a7ce507 [3] http://walfield.org/pub/people/neal/xgethostname/ [4] https://www.gnu.org/software/hurd/hurd/porting/guidelines.html#MAXHOSTNAMELEN_tt_