From: Omar Polo Subject: Re: gotwebd: finishing the template refactoring To: Omar Polo Cc: gameoftrees@openbsd.org Date: Fri, 10 Mar 2023 18:54:45 +0100 On 2023/02/21 20:19:32 +0100, Omar Polo wrote: > Now that we have everything under the templates, I'd like to close the > whole refactoring. The idea is to have only one entrypoint, > gotweb_render_page() that renders the template (function) given as > argument. > > The idea is to simplify gotweb_process_request() a bit more, handling > all the cases inside the big switch. It's also a nice preparation > step towards an incoming diff to have gotwebd return a non-200 HTTP > code on failure. > > The only real difference is that gotweb_render_index() now skips > directory entries which fails to handle. I don't feel too bad about > it. Unlike other parts of got, since gotwebd speaks HTTP, we need to > decide up until which point we care about errors and generate page > output only after that. In this case, I decided to care about the > failure in opening /var/www/got/public and not about failure in > reading its content. The follow-up diff regarding the HTTP status > code will make this clearer. > > as usual, my instance is running with this applied. friendly ping :) diffstat /home/op/w/gotd M gotwebd/config.c | 1+ 0- M gotwebd/fcgi.c | 1+ 0- M gotwebd/gotweb.c | 75+ 79- M gotwebd/gotwebd.c | 1+ 0- M gotwebd/gotwebd.h | 10+ 5- M gotwebd/pages.tmpl | 9+ 13- M gotwebd/parse.y | 3+ 1- M gotwebd/sockets.c | 1+ 0- 8 files changed, 101 insertions(+), 98 deletions(-) diff /home/op/w/gotd commit - 5add7f42e1397d136860680e1f0411db17b4f22c path + /home/op/w/gotd blob - ec88e1ef5acdb77906f2588283329e2a1f4eda21 file + gotwebd/config.c --- gotwebd/config.c +++ gotwebd/config.c @@ -37,6 +37,7 @@ #include #include "got_opentemp.h" +#include "got_reference.h" #include "proc.h" #include "gotwebd.h" blob - ba1803b45941d2c4b83b31f232b8e7edfb92df63 file + gotwebd/fcgi.c --- gotwebd/fcgi.c +++ gotwebd/fcgi.c @@ -33,6 +33,7 @@ #include #include "got_error.h" +#include "got_reference.h" #include "proc.h" #include "gotwebd.h" blob - 52aa037e4a8d734d17311aabbd7586d7e23d8fcd file + gotwebd/gotweb.c --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -85,7 +85,7 @@ static const struct got_error *gotweb_render_index(str char *); static const struct got_error *gotweb_assign_querystring(struct querystring **, char *, char *); -static const struct got_error *gotweb_render_index(struct request *); +static int gotweb_render_index(struct template *); static const struct got_error *gotweb_init_repo_dir(struct repo_dir **, const char *); static const struct got_error *gotweb_load_got_path(struct request *c, @@ -144,17 +144,12 @@ gotweb_process_request(struct request *c) gotweb_process_request(struct request *c) { const struct got_error *error = NULL, *error2 = NULL; - struct got_blob_object *blob = NULL; struct server *srv = NULL; struct querystring *qs = NULL; struct repo_dir *repo_dir = NULL; - struct got_reflist_head refs; - FILE *fp = NULL; uint8_t err[] = "gotwebd experienced an error: "; - int r, html = 0, fd = -1; + int r, html = 0; - TAILQ_INIT(&refs); - /* init the transport */ error = gotweb_init_transport(&c->t); if (error) { @@ -217,7 +212,8 @@ gotweb_process_request(struct request *c) if (error) goto done; - error2 = got_open_blob_for_output(&blob, &fd, &binary, c); + error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd, + &binary, c); if (error2) goto render; @@ -230,12 +226,12 @@ gotweb_process_request(struct request *c) goto done; for (;;) { - error = got_object_blob_read_block(&len, blob); + error = got_object_blob_read_block(&len, c->t->blob); if (error) goto done; if (len == 0) break; - buf = got_object_blob_get_read_buf(blob); + buf = got_object_blob_get_read_buf(c->t->blob); if (fcgi_gen_binary_response(c, buf, len) == -1) goto done; } @@ -259,7 +255,8 @@ gotweb_process_request(struct request *c) if (error) goto done; - error2 = got_open_blob_for_output(&blob, &fd, &binary, c); + error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd, + &binary, c); if (error2) goto render; if (binary) { @@ -289,9 +286,6 @@ render: goto done; html = 1; - if (gotweb_render_header(c->tp) == -1) - goto err; - if (error2) { error = error2; goto err; @@ -304,15 +298,15 @@ render: log_warnx("%s: %s", __func__, error->msg); goto err; } - if (gotweb_render_blame(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_blame) == -1) goto done; break; case BLOB: - if (gotweb_render_blob(c->tp, blob) == -1) + if (gotweb_render_page(c->tp, gotweb_render_blob) == -1) goto err; break; case BRIEFS: - if (gotweb_render_briefs(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_briefs) == -1) goto err; break; case COMMITS: @@ -321,7 +315,7 @@ render: log_warnx("%s: %s", __func__, error->msg); goto err; } - if (gotweb_render_commits(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_commits) == -1) goto err; break; case DIFF: @@ -330,23 +324,28 @@ render: log_warnx("%s: %s", __func__, error->msg); goto err; } - error = got_open_diff_for_output(&fp, &fd, c); + error = got_open_diff_for_output(&c->t->fp, &c->t->fd, c); if (error) { log_warnx("%s: %s", __func__, error->msg); goto err; } - if (gotweb_render_diff(c->tp, fp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_diff) == -1) goto err; break; case INDEX: - error = gotweb_render_index(c); - if (error) { - log_warnx("%s: %s", __func__, error->msg); + c->t->nrepos = scandir(srv->repos_path, &c->t->repos, NULL, + alphasort); + if (c->t->nrepos == -1) { + c->t->repos = NULL; + error = got_error_from_errno2("scandir", + srv->repos_path); goto err; } + if (gotweb_render_page(c->tp, gotweb_render_index) == -1) + goto err; break; case SUMMARY: - error = got_ref_list(&refs, c->t->repo, "refs/heads", + error = got_ref_list(&c->t->refs, c->t->repo, "refs/heads", got_ref_cmp_by_name, NULL); if (error) { log_warnx("%s: got_ref_list: %s", __func__, @@ -361,7 +360,7 @@ render: goto err; } qs->action = SUMMARY; - if (gotweb_render_summary(c->tp, &refs) == -1) + if (gotweb_render_page(c->tp, gotweb_render_summary) == -1) goto done; break; case TAG: @@ -375,7 +374,7 @@ render: "bad commit id"); goto err; } - if (gotweb_render_tag(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_tag) == -1) goto done; break; case TAGS: @@ -384,7 +383,7 @@ render: log_warnx("%s: %s", __func__, error->msg); goto err; } - if (gotweb_render_tags(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_tags) == -1) goto done; break; case TREE: @@ -393,7 +392,7 @@ render: log_warnx("%s: %s", __func__, error->msg); goto err; } - if (gotweb_render_tree(c->tp) == -1) + if (gotweb_render_page(c->tp, gotweb_render_tree) == -1) goto err; break; case ERR: @@ -421,21 +420,7 @@ done: if (html && fcgi_printf(c, "\n") == -1) return; done: - if (blob) - got_object_blob_close(blob); - if (fp) { - error = got_gotweb_flushfile(fp, fd); - if (error) - log_warnx("%s: got_gotweb_flushfile failure: %s", - __func__, error->msg); - fd = -1; - } - if (fd != -1) - close(fd); - if (html && srv != NULL) - gotweb_render_footer(c->tp); - - got_ref_list_free(&refs); + return; } struct server * @@ -474,6 +459,7 @@ gotweb_init_transport(struct transport **t) TAILQ_INIT(&(*t)->repo_commits); TAILQ_INIT(&(*t)->repo_tags); + TAILQ_INIT(&(*t)->refs); (*t)->repo = NULL; (*t)->repo_dir = NULL; @@ -483,6 +469,8 @@ gotweb_init_transport(struct transport **t) (*t)->next_disp = 0; (*t)->prev_disp = 0; + (*t)->fd = -1; + return error; } @@ -770,9 +758,12 @@ gotweb_free_transport(struct transport *t) void gotweb_free_transport(struct transport *t) { + const struct got_error *err; struct repo_commit *rc = NULL, *trc = NULL; struct repo_tag *rt = NULL, *trt = NULL; + int i; + got_ref_list_free(&t->refs); TAILQ_FOREACH_SAFE(rc, &t->repo_commits, entry, trc) { TAILQ_REMOVE(&t->repo_commits, rc, entry); gotweb_free_repo_commit(rc); @@ -786,6 +777,22 @@ gotweb_free_transport(struct transport *t) free(t->more_id); free(t->next_id); free(t->prev_id); + if (t->blob) + got_object_blob_close(t->blob); + if (t->fp) { + err = got_gotweb_flushfile(t->fp, t->fd); + if (err) + log_warnx("%s: got_gotweb_flushfile failure: %s", + __func__, err->msg); + t->fd = -1; + } + if (t->fd != -1) + close(t->fd); + if (t->repos) { + for (i = 0; i < t->nrepos; ++i) + free(t->repos[i]); + free(t->repos); + } free(t); } @@ -848,30 +855,24 @@ static const struct got_error * } } -static const struct got_error * -gotweb_render_index(struct request *c) +static int +gotweb_render_index(struct template *tp) { const struct got_error *error = NULL; + struct request *c = tp->tp_arg; struct server *srv = c->srv; struct transport *t = c->t; struct querystring *qs = t->qs; struct repo_dir *repo_dir = NULL; - struct dirent **sd_dent = NULL; - unsigned int d_cnt, d_i, d_disp = 0; + struct dirent **sd_dent = t->repos; + unsigned int d_i, d_disp = 0; unsigned int d_skipped = 0; - int type; + int type, r; - d_cnt = scandir(srv->repos_path, &sd_dent, NULL, alphasort); - if (d_cnt == -1) { - sd_dent = NULL; - error = got_error_from_errno2("scandir", srv->repos_path); - goto done; - } - if (gotweb_render_repo_table_hdr(c->tp) == -1) - goto done; + return -1; - for (d_i = 0; d_i < d_cnt; d_i++) { + for (d_i = 0; d_i < t->nrepos; d_i++) { if (srv->max_repos > 0 && t->prev_disp == srv->max_repos) break; @@ -884,7 +885,7 @@ gotweb_render_index(struct request *c) error = got_path_dirent_type(&type, srv->repos_path, sd_dent[d_i]); if (error) - goto done; + continue; if (type != DT_DIR) { d_skipped++; continue; @@ -898,50 +899,45 @@ gotweb_render_index(struct request *c) error = gotweb_init_repo_dir(&repo_dir, sd_dent[d_i]->d_name); if (error) - goto done; + continue; error = gotweb_load_got_path(c, repo_dir); - if (error && error->code == GOT_ERR_NOT_GIT_REPO) { - error = NULL; + if (error && error->code == GOT_ERR_LONELY_PACKIDX) { + if (error->code != GOT_ERR_NOT_GIT_REPO) + log_warnx("%s: %s: %s", __func__, + sd_dent[d_i]->d_name, error->msg); gotweb_free_repo_dir(repo_dir); repo_dir = NULL; d_skipped++; continue; } - if (error && error->code != GOT_ERR_LONELY_PACKIDX) - goto done; d_disp++; t->prev_disp++; - if (gotweb_render_repo_fragment(c->tp, repo_dir) == -1) - goto done; - + r = gotweb_render_repo_fragment(c->tp, repo_dir); gotweb_free_repo_dir(repo_dir); - repo_dir = NULL; + if (r == -1) + return -1; + t->next_disp++; if (d_disp == srv->max_repos_display) break; } - t->repos_total = d_cnt - d_skipped; + t->repos_total = t->nrepos - d_skipped; if (srv->max_repos_display == 0) - goto done; + return 0; if (srv->max_repos > 0 && srv->max_repos < srv->max_repos_display) - goto done; + return 0; if (t->repos_total <= srv->max_repos || t->repos_total <= srv->max_repos_display) - goto done; + return 0; if (gotweb_render_navs(c->tp) == -1) - goto done; -done: - if (sd_dent) { - for (d_i = 0; d_i < d_cnt; d_i++) - free(sd_dent[d_i]); - free(sd_dent); - } - return error; + return -1; + + return 0; } static inline int blob - cc0a4954a61a219297305919f2441a682fcb9a78 file + gotwebd/gotwebd.c --- gotwebd/gotwebd.c +++ gotwebd/gotwebd.c @@ -41,6 +41,7 @@ #include #include "got_opentemp.h" +#include "got_reference.h" #include "proc.h" #include "gotwebd.h" blob - 98e9f879c0f9fc272de90ddc649d0c9aa2d88875 file + gotwebd/gotwebd.h --- gotwebd/gotwebd.h +++ gotwebd/gotwebd.h @@ -186,6 +186,7 @@ struct transport { struct transport { TAILQ_HEAD(repo_commits_head, repo_commit) repo_commits; TAILQ_HEAD(repo_tags_head, repo_tag) repo_tags; + struct got_reflist_head refs; struct got_repository *repo; struct repo_dir *repo_dir; struct querystring *qs; @@ -196,6 +197,11 @@ struct transport { unsigned int next_disp; unsigned int prev_disp; unsigned int tag_count; + struct got_blob_object *blob; + int fd; + FILE *fp; + struct dirent **repos; + int nrepos; }; enum socket_priv_fds { @@ -463,20 +469,19 @@ int gotweb_render_header(struct template *); void gotweb_free_transport(struct transport *); /* pages.tmpl */ -int gotweb_render_header(struct template *); -int gotweb_render_footer(struct template *); +int gotweb_render_page(struct template *, int (*)(struct template *)); int gotweb_render_repo_table_hdr(struct template *); int gotweb_render_repo_fragment(struct template *, struct repo_dir *); int gotweb_render_briefs(struct template *); int gotweb_render_navs(struct template *); int gotweb_render_commits(struct template *); -int gotweb_render_blob(struct template *, struct got_blob_object *); +int gotweb_render_blob(struct template *); int gotweb_render_tree(struct template *); int gotweb_render_tags(struct template *); int gotweb_render_tag(struct template *); -int gotweb_render_diff(struct template *, FILE *); +int gotweb_render_diff(struct template *); int gotweb_render_branches(struct template *, struct got_reflist_head *); -int gotweb_render_summary(struct template *, struct got_reflist_head *); +int gotweb_render_summary(struct template *); int gotweb_render_blame(struct template *); int gotweb_render_rss(struct template *); blob - bea0461c4b1286b162ce1d279fb93dd847ff4ab5 file + gotwebd/pages.tmpl --- gotwebd/pages.tmpl +++ gotwebd/pages.tmpl @@ -54,7 +54,8 @@ static inline int rss_author(struct template *, char * !} -{{ define gotweb_render_header(struct template *tp) }} +{{ define gotweb_render_page(struct template *tp, + int (*body)(struct template *)) }} {! struct request *c = tp->tp_arg; struct server *srv = c->srv; @@ -110,13 +111,7 @@ static inline int rss_author(struct template *, char *
-{{ end }} - -{{ define gotweb_render_footer(struct template *tp) }} -{! - struct request *c = tp->tp_arg; - struct server *srv = c->srv; -!} + {{ render body(tp) }}
{{ if srv->show_site_owner }} @@ -433,11 +428,11 @@ static inline int rss_author(struct template *, char *
{{ end }} -{{ define gotweb_render_blob(struct template *tp, - struct got_blob_object *blob) }} +{{ define gotweb_render_blob(struct template *tp) }} {! struct request *c = tp->tp_arg; struct transport *t = c->t; + struct got_blob_object *blob = t->blob; struct repo_commit *rc = TAILQ_FIRST(&t->repo_commits); !}
@@ -711,10 +706,11 @@ static inline int rss_author(struct template *, char *
{{ end }} -{{ define gotweb_render_diff(struct template *tp, FILE *fp) }} +{{ define gotweb_render_diff(struct template *tp) }} {! struct request *c = tp->tp_arg; struct transport *t = c->t; + FILE *fp = t->fp; struct repo_commit *rc = TAILQ_FIRST(&t->repo_commits); char *line = NULL; size_t linesize = 0; @@ -851,12 +847,12 @@ static inline int rss_author(struct template *, char *
{{ end }} -{{ define gotweb_render_summary(struct template *tp, - struct got_reflist_head *refs) }} +{{ define gotweb_render_summary(struct template *tp) }} {! struct request *c = tp->tp_arg; struct server *srv = c->srv; struct transport *t = c->t; + struct got_reflist_head *refs = &t->refs; !}
{{ if srv->show_repo_description }} blob - cc7ccf7ef44f2f9a5f354959abcc57ee681162bf file + gotwebd/parse.y --- gotwebd/parse.y +++ gotwebd/parse.y @@ -47,9 +47,11 @@ #include #include +#include "got_sockaddr.h" +#include "got_reference.h" + #include "proc.h" #include "gotwebd.h" -#include "got_sockaddr.h" TAILQ_HEAD(files, file) files = TAILQ_HEAD_INITIALIZER(files); static struct file { blob - ecb88027e072a125a309c08b985733a77ab2a26d file + gotwebd/sockets.c --- gotwebd/sockets.c +++ gotwebd/sockets.c @@ -51,6 +51,7 @@ #include "got_error.h" #include "got_opentemp.h" +#include "got_reference.h" #include "got_repository.h" #include "proc.h"