Web lists-archives.com

Re: [PATCH v3 1/2] use HOST_NAME_MAX to size buffers for gethostname(2)




Am 19.04.2017 um 21:09 schrieb Torsten Bögershausen:
On 2017-04-19 19:28, René Scharfe wrote:
[]
One or two minor comments inline
diff --git a/builtin/gc.c b/builtin/gc.c
index 2daede7820..4c1c01e87d 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -228,21 +228,99 @@ static int need_to_gc(void)
  	return 1;
  }
+struct pidfile {
+	struct strbuf buf;
+	char *hostname;
+};
+
+#define PIDFILE_INIT { STRBUF_INIT }
+
+static void pidfile_release(struct pidfile *pf)
+{
+	pf->hostname = NULL;
+	strbuf_release(&pf->buf);
+}
+
+static int pidfile_read(struct pidfile *pf, const char *path,
+			unsigned int max_age_seconds)
+{
+	int fd;
+	struct stat st;
+	ssize_t len;
+	char *space;
+	int rc = -1;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return rc;
+
+	if (fstat(fd, &st))
+		goto out;
+	if (time(NULL) - st.st_mtime > max_age_seconds)
+		goto out;
+	if (st.st_size > (size_t)st.st_size)

Minor: we need xsize_t here ?
if (st.st_size > xsize_t(st.st_size))

No, xsize_t() would do the same check and die on overflow, and pidfile_read() is supposed to handle big pids gracefully.


+		goto out;
+
+	len = strbuf_read(&pf->buf, fd, st.st_size);
+	if (len < 0)
+		goto out;
+
+	space = strchr(pf->buf.buf, ' ');
+	if (!space) {
+		pidfile_release(pf);
+		goto out;
+	}
+	pf->hostname = space + 1;
+	*space = '\0';
+
+	rc = 0;
+out:
+	close(fd);
+	return rc;
+}
+
+static int parse_pid(const char *value, pid_t *ret)
+{
+	if (value && *value) {
+		char *end;
+		intmax_t val;
+
+		errno = 0;
+		val = strtoimax(value, &end, 0);
+		if (errno == ERANGE)
+			return 0;
+		if (*end)
+			return 0;
+		if (labs(val) > maximum_signed_value_of_type(pid_t)) {
+			errno = ERANGE;
+			return 0;
+		}
+		*ret = val;
+		return 1;
+	}
+	errno = EINVAL;
+	return 0;
+}
+
+static int pidfile_process_exists(const struct pidfile *pf)
+{
+	pid_t pid;
+	return parse_pid(pf->buf.buf, &pid) &&
+		(!kill(pid, 0) || errno == EPERM);
+}
+
  /* return NULL on success, else hostname running the gc */
-static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
+static int lock_repo_for_gc(int force, struct pidfile *pf)
  {
  	static struct lock_file lock;
  	char my_host[128];

Huh ?
should this be increased, may be in another path ?

It should, but not in this patch.

René