David Turner <dtur...@twosigma.com> writes:

> @@ -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).

Reply via email to