Download raw body.
gotwebd: linkables lines in blame page
Omar Polo <op@omarpolo.com> wrote:
> Tracey Emery <tracey@traceyemery.net> wrote:
> > On Tue, Aug 09, 2022 at 04:48:04PM +0200, Omar Polo wrote:
> > > P.S.: what do you think if we introduce something a printf-like
> > > fcgi_gen_responsef?
> > >
> >
> > Can you expound a bit here? What would it be used for? If it is used for
> > something like this, I think it would be a good idea and reduce lines:
> >
> > fcgi_gen_responsef(c, "<a href='#%s'>%s</a>", out_buff, out_buff);
> >
> > Is that what you mean?
>
> Yep, that's exactly what i had in mind. It could get rid of long
> sequences of code and make it easier to follow.
>
> I'll try to add it and see how it goes.
>
>
> Thanks,
Here's a draft for it.
The first `fcgi_printf' version i wrote tried to snprintf in the
output buffer first and falled back to asprintf as a second choice
only when the produced string was bigger than the buffer. It wasn't
worth the complexity thought, so here's a simpler version that just
builds a string with asprintf(3) and then feeds it to
fcgi_gen_binary_response. I doubt this will become a bottleneck.
I've started also to simplify some bits in got_operations.c to show
how it's like to have this function available; I've searched for the
bits that generates the links as they're one of the most pleasing part
to simplify. (truth to be told, we could also refactor the code to
generate a link into a separate function.)
I'm not 100% sure if it's correct, so please take a careful look. The
main difference is that fcgi_gen_response is a no-op when the given
string is NULL or "" but fcgi_printf, since relies on vasprintf, turns
NULL into "(null)". I'm not sure of exactly which fields may be NULL
and which will never be.
Thanks,
Omar Polo
diff /home/op/w/got
commit - bf80b15220f51490025e916633cdd70816113604
path + /home/op/w/got
blob - f6cb81448f5778d7872b537fd6b90838285c99ee
file + gotwebd/fcgi.c
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -25,6 +25,7 @@
#include <errno.h>
#include <event.h>
#include <imsg.h>
+#include <stdarg.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
@@ -308,6 +309,27 @@ fcgi_timeout(int fd, short events, void *arg)
}
int
+fcgi_printf(struct request *c, const char *fmt, ...)
+{
+ va_list ap;
+ char *str;
+ int r;
+
+ va_start(ap, fmt);
+ r = vasprintf(&str, fmt, ap);
+ va_end(ap);
+
+ if (r == -1) {
+ log_warn("%s: asprintf", __func__);
+ return -1;
+ }
+
+ r = fcgi_gen_binary_response(c, str, r);
+ free(str);
+ return r;
+}
+
+int
fcgi_gen_binary_response(struct request *c, const uint8_t *data, int len)
{
int r;
blob - 1c274b225d0e710d16528c4b9970348bb3bec74e
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -829,10 +829,11 @@ got_output_repo_tree(struct request *c)
struct got_reflist_head refs;
struct got_tree_object *tree = NULL;
struct repo_dir *repo_dir = t->repo_dir;
+ const char *name, *index_page_str, *folder;
char *id_str = NULL;
- char *path = NULL, *in_repo_path = NULL, *build_folder = NULL;
- char *modestr = NULL, *name = NULL;
- int nentries, i;
+ char *path = NULL, *in_repo_path = NULL;
+ char *modestr = NULL;
+ int nentries, i, r;
TAILQ_INIT(&refs);
@@ -871,6 +872,9 @@ got_output_repo_tree(struct request *c)
nentries = got_object_tree_get_nentries(tree);
+ index_page_str = qs->index_page_str ? qs->index_page_str : "";
+ folder = qs->folder ? qs->folder : "";
+
for (i = 0; i < nentries; i++) {
struct got_tree_entry *te;
mode_t mode;
@@ -917,53 +921,23 @@ got_output_repo_tree(struct request *c)
}
}
- name = strdup(got_tree_entry_get_name(te));
- if (name == NULL) {
- error = got_error_from_errno("strdup");
- goto done;
- }
if (S_ISDIR(mode)) {
- if (asprintf(&build_folder, "%s/%s",
- qs->folder ? qs->folder : "",
- got_tree_entry_get_name(te)) == -1) {
- error = got_error_from_errno("asprintf");
- goto done;
- }
-
if (fcgi_gen_response(c,
"<div class='tree_wrapper'>\n") == -1)
- goto done;
+ goto done;
if (fcgi_gen_response(c, "<div class='tree_line'>") == -1)
goto done;
- if (fcgi_gen_response(c, "<a class='diff_directory' "
- "href='?index_page=") == -1)
+ name = got_tree_entry_get_name(te);
+ r = fcgi_printf(c,
+ "<a class='diff_directory' "
+ "href='?index_page=%s&path=%s&action=tree"
+ "&commit=%s&folder=%s/%s'>%s%s</a>",
+ index_page_str, qs->path, rc->commit_id,
+ folder, name, name, modestr);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, qs->index_page_str) == -1)
- goto done;
- if (fcgi_gen_response(c, "&path=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->path) == -1)
- goto done;
- if (fcgi_gen_response(c, "&action=tree") == -1)
- goto done;
- if (fcgi_gen_response(c, "&commit=") == -1)
- goto done;
- if (fcgi_gen_response(c, rc->commit_id) == -1)
- goto done;
- if (fcgi_gen_response(c, "&folder=") == -1)
- goto done;
- if (fcgi_gen_response(c, build_folder) == -1)
- goto done;
- if (fcgi_gen_response(c, "'>") == -1)
- goto done;
- if (fcgi_gen_response(c, name) == -1)
- goto done;
- if (fcgi_gen_response(c, modestr) == -1)
- goto done;
- if (fcgi_gen_response(c, "</a>") == -1)
- goto done;
if (fcgi_gen_response(c, "</div>\n") == -1)
goto done;
@@ -979,144 +953,44 @@ got_output_repo_tree(struct request *c)
goto done;
} else {
- free(name);
- name = strdup(got_tree_entry_get_name(te));
- if (name == NULL) {
- error = got_error_from_errno("strdup");
- goto done;
- }
-
if (fcgi_gen_response(c,
"<div class='tree_wrapper'>\n") == -1)
goto done;
if (fcgi_gen_response(c, "<div class='tree_line'>") == -1)
goto done;
- if (fcgi_gen_response(c,
- "<a href='?index_page=") == -1)
+ name = got_tree_entry_get_name(te);
+ r = fcgi_printf(c,
+ "<a href='?index_page=%s&path=%s&action=blob"
+ "&commit=%s&folder=%s&file=%s'>%s%s</a>",
+ index_page_str, qs->path, rc->commit_id,
+ folder, name, name, modestr);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, qs->index_page_str) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&path=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->path) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&action=blob") == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&commit=") == -1)
- goto done;
- if (fcgi_gen_response(c, rc->commit_id) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&folder=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->folder) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&file=") == -1)
- goto done;
- if (fcgi_gen_response(c, name) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "'>") == -1)
- goto done;
- if (fcgi_gen_response(c, name) == -1)
- goto done;
- if (fcgi_gen_response(c, modestr) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "</a>") == -1)
- goto done;
-
if (fcgi_gen_response(c, "</div>\n") == -1)
goto done;
if (fcgi_gen_response(c, "<div class='tree_line_blank'>") == -1)
goto done;
- if (fcgi_gen_response(c,
- "<a href='?index_page=") == -1)
+ r = fcgi_printf(c,
+ "<a href='?index_page=%s&path=%s&action=commits"
+ "&commit=%s&folder=%s&file=%s'>commits</a>\n",
+ index_page_str, qs->path, rc->commit_id,
+ folder, name);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, qs->index_page_str) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&path=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->path) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&action=commits") == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&commit=") == -1)
- goto done;
- if (fcgi_gen_response(c, rc->commit_id) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&folder=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->folder) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&file=") == -1)
- goto done;
- if (fcgi_gen_response(c, name) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "'>") == -1)
- goto done;
-
- if (fcgi_gen_response(c, "commits") == -1)
- goto done;
- if (fcgi_gen_response(c, "</a>\n") == -1)
- goto done;
-
if (fcgi_gen_response(c, " | \n") == -1)
goto done;
- if (fcgi_gen_response(c,
- "<a href='?index_page=") == -1)
- goto done;
+ r = fcgi_printf(c,
+ "<a href='?index_page=%s&path=%s&action=blame"
+ "&commit=%s&folder=%s&file=%s'>blame</a>\n",
+ index_page_str, qs->path, rc->commit_id,
+ folder, name);
- if (fcgi_gen_response(c, qs->index_page_str) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&path=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->path) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&action=blame") == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&commit=") == -1)
- goto done;
- if (fcgi_gen_response(c, rc->commit_id) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&folder=") == -1)
- goto done;
- if (fcgi_gen_response(c, qs->folder) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "&file=") == -1)
- goto done;
- if (fcgi_gen_response(c, name) == -1)
- goto done;
-
- if (fcgi_gen_response(c, "'>") == -1)
- goto done;
-
- if (fcgi_gen_response(c, "blame") == -1)
- goto done;
- if (fcgi_gen_response(c, "</a>\n") == -1)
- goto done;
-
if (fcgi_gen_response(c, "</div>\n") == -1)
goto done;
if (fcgi_gen_response(c, "</div>\n") == -1)
@@ -1124,19 +998,13 @@ got_output_repo_tree(struct request *c)
}
free(id_str);
id_str = NULL;
- free(build_folder);
- build_folder = NULL;
- free(name);
- name = NULL;
free(modestr);
modestr = NULL;
}
done:
free(id_str);
- free(build_folder);
free(modestr);
free(path);
- free(name);
got_ref_list_free(&refs);
if (commit)
got_object_commit_close(commit);
@@ -1286,11 +1154,13 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
struct transport *t = c->t;
struct querystring *qs = t->qs;
struct repo_dir *repo_dir = t->repo_dir;
+ const char *index_page_str;
char *line = NULL, *eline = NULL;
size_t linesize = 0;
off_t offset;
struct tm tm;
time_t committer_time;
+ int r;
if (nlines != a->nlines ||
(lineno != -1 && lineno < 1) || lineno > a->nlines)
@@ -1334,10 +1204,10 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
goto done;
}
+ index_page_str = qs->index_page_str ? qs->index_page_str : "";
+
while (bline->annotated) {
- int out_buff_size = 100;
char *smallerthan, *at, *nl, *committer;
- char out_buff[out_buff_size];
size_t len;
if (getline(&line, &linesize, a->f) == -1) {
@@ -1363,53 +1233,32 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
if (fcgi_gen_response(c, "<div class='blame_wrapper'>") == -1)
goto done;
- if (fcgi_gen_response(c, "<div class='blame_number'>") == -1)
+
+ r = fcgi_printf(c, "<div class='blame_number'>%.*d</div>",
+ a->nlines_prec, a->lineno_cur);
+ if (r == -1)
goto done;
- if (snprintf(out_buff, strlen(out_buff), "%.*d", a->nlines_prec,
- a->lineno_cur) < 0)
- goto done;
- if (fcgi_gen_response(c, out_buff) == -1)
- goto done;
- if (fcgi_gen_response(c, "</div>") == -1)
- goto done;
if (fcgi_gen_response(c, "<div class='blame_hash'>") == -1)
goto done;
- if (fcgi_gen_response(c, "<a href='?index_page=") == -1)
+ r = fcgi_printf(c,
+ "<a href='?index_page=%s&path=%s&action=diff&commit=%s'>"
+ "%.8s</a></div>",
+ index_page_str, repo_dir->name, bline->id_str,
+ bline->id_str);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, qs->index_page_str) == -1)
- goto done;
- if (fcgi_gen_response(c, "&path=") == -1)
- goto done;
- if (fcgi_gen_response(c, repo_dir->name) == -1)
- goto done;
- if (fcgi_gen_response(c, "&action=diff&commit=") == -1)
- goto done;
- if (fcgi_gen_response(c, bline->id_str) == -1)
- goto done;
- if (fcgi_gen_response(c, "'>") == -1)
- goto done;
- if (snprintf(out_buff, 10, "%.8s", bline->id_str) < 0)
- goto done;
- if (fcgi_gen_response(c, out_buff) == -1)
- goto done;
- if (fcgi_gen_response(c, "</a></div>") == -1)
- goto done;
- if (fcgi_gen_response(c, "<div class='blame_date'>") == -1)
+ r = fcgi_printf(c, "<div class='blame_date'>%s</div>",
+ bline->datebuf);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, bline->datebuf) == -1)
- goto done;
- if (fcgi_gen_response(c, "</div>") == -1)
- goto done;
- if (fcgi_gen_response(c, "<div class='blame_author'>") == -1)
+ r = fcgi_printf(c, "<div class='blame_author'>%s</div>",
+ committer);
+ if (r == -1)
goto done;
- if (fcgi_gen_response(c, committer) == -1)
- goto done;
- if (fcgi_gen_response(c, "</div>") == -1)
- goto done;
if (fcgi_gen_response(c, "<div class='blame_code'>") == -1)
goto done;
blob - b58b1e7be0421dac9ff6c9717bacb59208ad1bbb
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -438,6 +438,9 @@ void fcgi_timeout(int, short, void *);
void fcgi_cleanup_request(struct request *);
void fcgi_create_end_record(struct request *);
void dump_fcgi_record(const char *, struct fcgi_record_header *);
+int fcgi_printf(struct request *, const char *, ...)
+ __attribute__((__format__(printf, 2, 3)))
+ __attribute__((__nonnull__(2)));
int fcgi_gen_response(struct request *, const char *);
int fcgi_gen_binary_response(struct request *, const uint8_t *, int);
gotwebd: linkables lines in blame page