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