git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] xgethostname: handle long hostnames

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 19, 2017, 01:49 UTC
Message-ID
<xmqqpog9cgkq.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<281d0843-d48a-b7ab-737b-b9528689d44e@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 5 quoted lines
> 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.
Show 44 quoted lines
>
> 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;
>  		}
>  	}
Previous: Junio C Hamano
Message 9 of 9 in “xgethostname: handle long hostnames”
  1. xgethostname: handle long hostnamesDavid Turner, Apr 17, 2017
  2. Junio C HamanoApr 18, 2017
  3. Junio C HamanoApr 18, 2017
  4. René ScharfeApr 18, 2017
  5. Jeff KingApr 18, 2017
  6. Junio C HamanoApr 19, 2017
  7. David TurnerApr 18, 2017
  8. Junio C HamanoApr 19, 2017
  9. Junio C HamanoApr 19, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.