> -----Original Message----- > From: René Scharfe [mailto:l.s.r@xxxxxx] > Sent: Tuesday, April 18, 2017 12:08 PM > To: Junio C Hamano <gitster@xxxxxxxxx>; 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