RE: [PATCH v2] xgethostname: handle long hostnames

[David Turner <David.Turner@twosigma.com>]

> -----Original Message-----
> From: René Scharfe [mailto:l.s.r@web.de]
> Sent: Tuesday, April 18, 2017 12:08 PM
> To: Junio C Hamano <gitster@pobox.com>; David Turner
... 
> >> Of course, my_host is sized to HOST_NAME_MAX + 1 and we are comparing
> >> it with locking_host, so perhaps we'd need to take this version to
> >> size locking_host to also HOST_NAME_MAX + 1, and then scan with %255c
> >> (but then shouldn't we scan with %256c instead?  I am not sure where
> >> these +1 comes from).
> >
> > That is, something along this line...
> >
> >   builtin/gc.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/gc.c b/builtin/gc.c index be75508292..4f85610d87
> > 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -240,7 +240,11 @@ static const char *lock_repo_for_gc(int force, pid_t*
> ret_pid)
> >   				       LOCK_DIE_ON_ERROR);
> >   	if (!force) {
> >   		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 =
> > @@ -256,7 +260,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 &&
> >   			/* be gentle to concurrent "gc" on remote hosts */
> >   			(strcmp(locking_host, my_host) || !kill(pid, 0) || errno
> == EPERM);
> >   		if (fp != NULL)
> >
> 
> How important is it to scan the whole file in one call?  We could split it up like
> this and use a strbuf to handle host names of any length.  We need to be
> permissive here to allow machines with different values for HOST_NAME_MAX
> to work with the same file on a network file system, so this would have to be the
> first patch, right?

If the writer has the smaller HOST_NAME_MAX, this will work fine.  If the reader
has the smaller HOST_NAME_MAX, and the writer's actual value is too long,
then there's no way the strcmp would succeed anyway.  So I don't think we need
to worry about it.

> NB: That && cascade has enough meat for a whole function.

+1