Re: [PATCH v2] xgethostname: handle long hostnames
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 18, 2017, 01:19 UTC
- Message-ID
- <xmqq1ssqikc5.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170417161748.31231-1-dturner@twosigma.com>
David Turner <dturner@twosigma.com> writes:
Show 8 quoted lines
> @@ -250,14 +250,14 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
> ...
> if (!force) {
> - static char locking_host[128];
> + static char locking_host[HOST_NAME_MAX + 1];
> int should_exit;
> fp = fopen(pidfile_path, "r");
> memset(locking_host, 0, sizeof(locking_host));I compared the result of applying this v2 directly on top of master and applying René's "Use HOST_NAME_MAX"and then applying your v1. This hunk is the only difference.
As this locking_host is used like so in the later part of the code:
time(NULL) - st.st_mtime <= 12 * 3600 && fscanf(fp, "%"SCNuMAX" %127c", &pid, locking_host) == 2 && /* be gentle to concurrent "gc" on remote hosts */ (strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM);
I suspect that turning it to HOST_NAME_MAX + 1 without tweaking the format "%127c" gives us an inconsistent resulting code.
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).