On 2022/11/09 19:54:41 +0100, Omar Polo <op@omarpolo.com> wrote:
> diff below does two things:
> 
>  - drops the unused path argument to got_get_repo_{owner,age}
> 
>  - makes gotweb_get_repo_{description,clone_url} use openat instead of
>    re-constructing the path.  less strings allocated on-the-fly.

Here's a reiteration of the previous diff.  Since i was already using
openat(2) in gotweb_get_repo_{description,clone_url} i thought i could
avoid the FILE handling at all: all we need to do is seek a bit to see
how big the file is and read from it.  It also changes len to be off_t
since now it's using lseek instead of fseek.

There is also another small change bundled.
gotweb_get_repo_description and clone_url differs in how they handle
the "len == 0" case when reading the file.  One sets the return to "",
the other one leaves NULL.  I'm normalizing the behaviour to return
NULL in both functions.

i thought of splitting it into two, but since dropping the unused path
argument part is very small i'm keeping everything in one diff, will
separate when committing.

ok?

diff /home/op/w/got
commit - 906ae6c0c06930fb1fc983786177f7de6c4d0403
path + /home/op/w/got
blob - b5a140ba22d460fe720b01f0db1a2c26b2d0661f
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -108,7 +108,7 @@ got_get_repo_owner(char **owner, struct request *c, ch
 }
 
 const struct got_error *
-got_get_repo_owner(char **owner, struct request *c, char *dir)
+got_get_repo_owner(char **owner, struct request *c)
 {
 	struct server *srv = c->srv;
 	struct transport *t = c->t;
@@ -134,7 +134,7 @@ got_get_repo_age(char **repo_age, struct request *c, c
 }
 
 const struct got_error *
-got_get_repo_age(char **repo_age, struct request *c, char *dir,
+got_get_repo_age(char **repo_age, struct request *c,
     const char *refname, int ref_tm)
 {
 	const struct got_error *error = NULL;
blob - 09d8f166fab40a704104d07aaa4c8e0564295c66
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -28,6 +28,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <event.h>
+#include <fcntl.h>
 #include <imsg.h>
 #include <sha1.h>
 #include <stdio.h>
@@ -93,9 +94,9 @@ static const struct got_error *gotweb_get_repo_descrip
 static const struct got_error *gotweb_load_got_path(struct request *c,
     struct repo_dir *);
 static const struct got_error *gotweb_get_repo_description(char **,
-    struct server *, char *);
+    struct server *, int);
 static const struct got_error *gotweb_get_clone_url(char **, struct server *,
-    char *);
+    int);
 static const struct got_error *gotweb_render_navs(struct request *);
 static const struct got_error *gotweb_render_blame(struct request *);
 static const struct got_error *gotweb_render_briefs(struct request *);
@@ -1545,8 +1546,7 @@ gotweb_render_branches(struct request *c)
 		if (strncmp(refname, "refs/heads/", 11) != 0)
 			continue;
 
-		error = got_get_repo_age(&age, c, qs->path, refname,
-		    TM_DIFF);
+		error = got_get_repo_age(&age, c, refname, TM_DIFF);
 		if (error)
 			goto done;
 
@@ -2431,7 +2431,6 @@ done:
 		goto err;
 	}
 
-
 	repo = find_cached_repo(srv, repo_dir->path);
 	if (repo == NULL) {
 		error = cache_repo(&repo, srv, repo_dir, sock);
@@ -2440,17 +2439,16 @@ done:
 	}
 	t->repo = repo;
 	error = gotweb_get_repo_description(&repo_dir->description, srv,
-	    repo_dir->path);
+	    dirfd(dt));
 	if (error)
 		goto err;
-	error = got_get_repo_owner(&repo_dir->owner, c, repo_dir->path);
+	error = got_get_repo_owner(&repo_dir->owner, c);
 	if (error)
 		goto err;
-	error = got_get_repo_age(&repo_dir->age, c, repo_dir->path,
-	    NULL, TM_DIFF);
+	error = got_get_repo_age(&repo_dir->age, c, NULL, TM_DIFF);
 	if (error)
 		goto err;
-	error = gotweb_get_clone_url(&repo_dir->url, srv, repo_dir->path);
+	error = gotweb_get_clone_url(&repo_dir->url, srv, dirfd(dt));
 err:
 	free(dir_test);
 	if (dt != NULL && closedir(dt) == EOF && error == NULL)
@@ -2483,105 +2481,88 @@ gotweb_get_repo_description(char **description, struct
 }
 
 static const struct got_error *
-gotweb_get_repo_description(char **description, struct server *srv, char *dir)
+gotweb_get_repo_description(char **description, struct server *srv, int dir)
 {
 	const struct got_error *error = NULL;
-	FILE *f = NULL;
-	char *d_file = NULL;
-	unsigned int len;
-	size_t n;
+	int fd = -1;
+	off_t len;
 
 	*description = NULL;
 	if (srv->show_repo_description == 0)
 		return NULL;
 
-	if (asprintf(&d_file, "%s/description", dir) == -1)
-		return got_error_from_errno("asprintf");
-
-	f = fopen(d_file, "r");
-	if (f == NULL) {
+	fd = openat(dir, "description", O_RDONLY);
+	if (fd == -1) {
 		if (errno != ENOENT && errno != EACCES)
-			error = got_error_from_errno2("fopen", d_file);
+			error = got_error_from_errno2("openat", "description");
 		goto done;
 	}
 
-	if (fseek(f, 0, SEEK_END) == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
-		goto done;
-	}
-	len = ftell(f);
+	len = lseek(fd, 0, SEEK_END);
 	if (len == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
+		error = got_error_from_errno("lseek");
 		goto done;
 	}
 
-	if (len == 0) {
-		*description = strdup("");
-		if (*description == NULL)
-			error = got_error_from_errno("strdup");
+	if (lseek(fd, 0, SEEK_SET) == -1) {
+		error = got_error_from_errno("fseek");
 		goto done;
 	}
 
-	if (fseek(f, 0, SEEK_SET) == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
+	if (len == 0)
 		goto done;
-	}
+
+	if (len > SIZE_MAX - 1)
+		len = SIZE_MAX - 1;
+
 	*description = calloc(len + 1, sizeof(**description));
 	if (*description == NULL) {
 		error = got_error_from_errno("calloc");
 		goto done;
 	}
 
-	n = fread(*description, 1, len, f);
-	if (n == 0 && ferror(f))
-		error = got_ferror(f, GOT_ERR_IO);
+	if (read(fd, *description, len) == -1)
+		error = got_error_from_errno("read");
 done:
-	if (f != NULL && fclose(f) == EOF && error == NULL)
-		error = got_error_from_errno("fclose");
-	free(d_file);
+	if (fd != -1 && close(fd) == -1 && error == NULL)
+		error = got_error_from_errno("close");
 	return error;
 }
 
 static const struct got_error *
-gotweb_get_clone_url(char **url, struct server *srv, char *dir)
+gotweb_get_clone_url(char **url, struct server *srv, int dir)
 {
 	const struct got_error *error = NULL;
-	FILE *f;
-	char *d_file = NULL;
-	unsigned int len;
-	size_t n;
+	int fd = -1;
+	off_t len;
 
 	*url = NULL;
-
 	if (srv->show_repo_cloneurl == 0)
 		return NULL;
 
-	if (asprintf(&d_file, "%s/cloneurl", dir) == -1)
-		return got_error_from_errno("asprintf");
-
-	f = fopen(d_file, "r");
-	if (f == NULL) {
+	fd = openat(dir, "cloneurl", O_RDONLY);
+	if (fd == -1) {
 		if (errno != ENOENT && errno != EACCES)
-			error = got_error_from_errno2("fopen", d_file);
+			error = got_error_from_errno2("openat", "cloneurl");
 		goto done;
 	}
 
-	if (fseek(f, 0, SEEK_END) == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
-		goto done;
-	}
-	len = ftell(f);
+	len = lseek(fd, 0, SEEK_END);
 	if (len == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
+		error = got_error_from_errno("lseek");
 		goto done;
 	}
+
+	if (lseek(fd, 0, SEEK_SET) == -1) {
+		error = got_error_from_errno("lseek");
+		goto done;
+	}
+
 	if (len == 0)
 		goto done;
 
-	if (fseek(f, 0, SEEK_SET) == -1) {
-		error = got_ferror(f, GOT_ERR_IO);
-		goto done;
-	}
+	if (len > SIZE_MAX - 1)
+		len = SIZE_MAX - 1;
 
 	*url = calloc(len + 1, sizeof(**url));
 	if (*url == NULL) {
@@ -2589,13 +2570,11 @@ gotweb_get_clone_url(char **url, struct server *srv, c
 		goto done;
 	}
 
-	n = fread(*url, 1, len, f);
-	if (n == 0 && ferror(f))
-		error = got_ferror(f, GOT_ERR_IO);
+	if (read(fd, *url, len) == -1)
+		error = got_error_from_errno("read");
 done:
-	if (f != NULL && fclose(f) == EOF && error == NULL)
+	if (fd != -1 && close(fd) == -1 && error == NULL)
 		error = got_error_from_errno("fclose");
-	free(d_file);
 	return error;
 }
 
blob - a8a55276acbd0b209205938c75b88d6918b1c6b8
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -455,8 +455,8 @@ const struct got_error *got_get_repo_owner(char **, st
 int fcgi_gen_binary_response(struct request *, const uint8_t *, int);
 
 /* got_operations.c */
-const struct got_error *got_get_repo_owner(char **, struct request *, char *);
-const struct got_error *got_get_repo_age(char **, struct request *, char *,
+const struct got_error *got_get_repo_owner(char **, struct request *);
+const struct got_error *got_get_repo_age(char **, struct request *,
     const char *, int);
 const struct got_error *got_get_repo_commits(struct request *, int);
 const struct got_error *got_get_repo_tags(struct request *, int);