Re: [PATCH v2] xgethostname: handle long hostnames
From: Junio C Hamano <hidden>
Date: 2017-04-19 01:49:22
René Scharfe [off-list ref] writes:
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?
Absolutely. FWIW, I agree with Peff that we do not need to use fscanf here; just reading a line into strbuf and picking pieces would be sufficient.
NB: That && cascade has enough meat for a whole function.
True, too.
quoted hunk
René --- builtin/gc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)diff --git a/builtin/gc.c b/builtin/gc.c index 1fca84c19d..d5e880028e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c@@ -251,10 +251,9 @@ 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 struct strbuf locking_host = STRBUF_INIT; int should_exit; fp = fopen(pidfile_path, "r"); - memset(locking_host, 0, sizeof(locking_host)); should_exit = fp != NULL && !fstat(fileno(fp), &st) &&@@ -268,9 +267,10 @@ 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, "%"SCNuMAX" ", &pid) == 1 && + !strbuf_getwholeline(&locking_host, fp, '\0') && /* be gentle to concurrent "gc" on remote hosts */ - (strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM); + (strcmp(locking_host.buf, my_host) || !kill(pid, 0) || errno == EPERM); if (fp != NULL) fclose(fp); if (should_exit) {@@ -278,7 +278,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) rollback_lock_file(&lock); *ret_pid = pid; free(pidfile_path); - return locking_host; + return locking_host.buf; } }