Download raw body.
gotwebd: error path leak & semplification for render_index
On 2022/11/09 15:48:47 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> On Wed, Nov 09, 2022 at 03:27:52PM +0100, Omar Polo wrote:
> > - strcmp(sd_dent[d_i]->d_name, "..") == 0)
> > + strcmp(sd_dent[d_i]->d_name, "..") == 0 ||
> > + sd_dent[d_i]->d_type != DT_DIR) {
>
> Unfortunately, we cannot rely on d_type directly. It is unreliable
> because it depends on the filesystem implementation.
> See got_path_dirent_type() in lib/path.c for a workaround.
Indeed, i forgot about the NFS without readdir plus issue. Here's an
updated diff that uses got_path_dirent_type.
diff /home/op/w/got
commit - 86979eca9f6d38458e13c64618b71e8667ce5b94
path + /home/op/w/got
blob - 7cb4b4aa6c9a239aeb25328ef01b302f2131753a
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -993,10 +993,9 @@ gotweb_render_index(struct request *c)
struct repo_dir *repo_dir = NULL;
DIR *d;
struct dirent **sd_dent = NULL;
- char *c_path = NULL;
- struct stat st;
unsigned int d_cnt, d_i, d_disp = 0;
- int r;
+ unsigned int d_skipped = 0;
+ int r, type;
d = opendir(srv->repos_path);
if (d == NULL) {
@@ -1011,25 +1010,6 @@ gotweb_render_index(struct request *c)
goto done;
}
- /* get total count of repos */
- for (d_i = 0; d_i < d_cnt; d_i++) {
- if (strcmp(sd_dent[d_i]->d_name, ".") == 0 ||
- strcmp(sd_dent[d_i]->d_name, "..") == 0)
- continue;
-
- if (asprintf(&c_path, "%s/%s", srv->repos_path,
- sd_dent[d_i]->d_name) == -1) {
- error = got_error_from_errno("asprintf");
- goto done;
- }
-
- if (lstat(c_path, &st) == 0 && S_ISDIR(st.st_mode) &&
- !got_path_dir_is_empty(c_path))
- t->repos_total++;
- free(c_path);
- c_path = NULL;
- }
-
r = fcgi_printf(c, "<div id='index_header'>\n"
"<div id='index_header_project'>Project</div>\n");
if (r == -1)
@@ -1051,13 +1031,24 @@ gotweb_render_index(struct request *c)
goto done;
for (d_i = 0; d_i < d_cnt; d_i++) {
- if (srv->max_repos > 0 && (d_i - 2) == srv->max_repos)
- break; /* account for parent and self */
+ if (srv->max_repos > 0 && t->prev_disp == srv->max_repos)
+ break;
if (strcmp(sd_dent[d_i]->d_name, ".") == 0 ||
- strcmp(sd_dent[d_i]->d_name, "..") == 0)
+ strcmp(sd_dent[d_i]->d_name, "..") == 0) {
+ d_skipped++;
continue;
+ }
+ error = got_path_dirent_type(&type, srv->repos_path,
+ sd_dent[d_i]);
+ if (error)
+ goto done;
+ if (type != DT_DIR) {
+ d_skipped++;
+ continue;
+ }
+
if (qs->index_page > 0 && (qs->index_page *
srv->max_repos_display) > t->prev_disp) {
t->prev_disp++;
@@ -1071,21 +1062,14 @@ gotweb_render_index(struct request *c)
error = gotweb_load_got_path(c, repo_dir);
if (error && error->code == GOT_ERR_NOT_GIT_REPO) {
error = NULL;
- continue;
- }
- else if (error && error->code != GOT_ERR_LONELY_PACKIDX)
- goto done;
-
- if (lstat(repo_dir->path, &st) == 0 &&
- S_ISDIR(st.st_mode) &&
- !got_path_dir_is_empty(repo_dir->path))
- goto render;
- else {
gotweb_free_repo_dir(repo_dir);
repo_dir = NULL;
+ d_skipped++;
continue;
}
-render:
+ if (error && error->code != GOT_ERR_LONELY_PACKIDX)
+ goto done;
+
d_disp++;
t->prev_disp++;
@@ -1201,6 +1185,8 @@ render:
if (d_disp == srv->max_repos_display)
break;
}
+ t->repos_total = d_cnt - d_skipped;
+
if (srv->max_repos_display == 0)
goto done;
if (srv->max_repos > 0 && srv->max_repos < srv->max_repos_display)
gotwebd: error path leak & semplification for render_index