From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: simplify blob/blobraw/rss page handling
To: gameoftrees@openbsd.org, Omar Polo <op@omarpolo.com>
Date: Sat, 01 Apr 2023 08:19:22 -0600

On March 31, 2023 8:13:43 AM MDT, Omar Polo <op@omarpolo.com> wrote:
>On 2023/03/11 16:43:59 +0100, Omar Polo <op@omarpolo.com> wrote:
>> Instead of special-casing BLOB, BLOBRAW and RSS, handle those pages in
>> the big switch as well.  Also, refactor the blob opening for
>> BLOB/BLOBRAW in a single place.
>> 
>> the diff isn't particularly pretty to read as is just shuffling some
>> code around for the most part.
>> 
>> ok?

No testing time but looks fine on a quick read.
Ok

>
>ping
>
>the code shuffling is just to handle all pages in the same switch,
>which makes the function easier to read imho.  it also saves some
>lines :-)
>
>> diffstat /home/op/w/gotwebd
>>  M  gotwebd/gotweb.c  |  47+  65-
>> 
>> 1 file changed, 47 insertions(+), 65 deletions(-)
>> 
>> diff /home/op/w/gotwebd
>> commit - 8f37175d25c1d0451e8296399000433f716d8218
>> path + /home/op/w/gotwebd
>> blob - d7c46fec7c87d40438428eec977f6b44df1daed1
>> file + gotwebd/gotweb.c
>> --- gotwebd/gotweb.c
>> +++ gotwebd/gotweb.c
>> @@ -147,6 +147,10 @@ gotweb_process_request(struct request *c)
>>  	struct server *srv = NULL;
>>  	struct querystring *qs = NULL;
>>  	struct repo_dir *repo_dir = NULL;
>> +	const char *rss_ctype = "application/rss+xml;charset=utf-8";
>> +	const uint8_t *buf;
>> +	size_t len;
>> +	int r, binary = 0;
>>  
>>  	/* init the transport */
>>  	error = gotweb_init_transport(&c->t);
>> @@ -201,11 +205,7 @@ gotweb_process_request(struct request *c)
>>  			goto err;
>>  	}
>>  
>> -	if (qs->action == BLOBRAW) {
>> -		const uint8_t *buf;
>> -		size_t len;
>> -		int binary, r;
>> -
>> +	if (qs->action == BLOBRAW || qs->action == BLOB) {
>>  		error = got_get_repo_commits(c, 1);
>>  		if (error)
>>  			goto err;
>> @@ -214,7 +214,40 @@ gotweb_process_request(struct request *c)
>>  		    &binary, c);
>>  		if (error)
>>  			goto err;
>> +	}
>>  
>> +	switch(qs->action) {
>> +	case BLAME:
>> +		error = got_get_repo_commits(c, 1);
>> +		if (error) {
>> +			log_warnx("%s: %s", __func__, error->msg);
>> +			goto err;
>> +		}
>> +		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>> +			return;
>> +		gotweb_render_page(c->tp, gotweb_render_blame);
>> +		return;
>> +	case BLOB:
>> +		if (binary) {
>> +			struct gotweb_url url = {
>> +				.index_page = -1,
>> +				.page = -1,
>> +				.action = BLOBRAW,
>> +				.path = qs->path,
>> +				.commit = qs->commit,
>> +				.folder = qs->folder,
>> +				.file = qs->file,
>> +			};
>> +
>> +			gotweb_reply(c, 302, NULL, &url);
>> +			return;
>> +		}
>> +
>> +		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>> +			return;
>> +		gotweb_render_page(c->tp, gotweb_render_blob);
>> +		return;
>> +	case BLOBRAW:
>>  		if (binary)
>>  			r = gotweb_reply_file(c, "application/octet-stream",
>>  			    qs->file, NULL);
>> @@ -233,67 +266,7 @@ gotweb_process_request(struct request *c)
>>  			if (fcgi_gen_binary_response(c, buf, len) == -1)
>>  				break;
>>  		}
>> -
>>  		return;
>> -	}
>> -
>> -	if (qs->action == BLOB) {
>> -		int binary;
>> -		struct gotweb_url url = {
>> -			.index_page = -1,
>> -			.page = -1,
>> -			.action = BLOBRAW,
>> -			.path = qs->path,
>> -			.commit = qs->commit,
>> -			.folder = qs->folder,
>> -			.file = qs->file,
>> -		};
>> -
>> -		error = got_get_repo_commits(c, 1);
>> -		if (error)
>> -			goto err;
>> -
>> -		error = got_open_blob_for_output(&c->t->blob, &c->t->fd,
>> -		    &binary, c);
>> -		if (error)
>> -			goto err;
>> -		if (binary) {
>> -			gotweb_reply(c, 302, NULL, &url);
>> -			return;
>> -		}
>> -	}
>> -
>> -	if (qs->action == RSS) {
>> -		const char *ctype = "application/rss+xml;charset=utf-8";
>> -
>> -		if (gotweb_reply_file(c, ctype, repo_dir->name, ".rss") == -1)
>> -			return;
>> -
>> -		error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
>> -		if (error) {
>> -			log_warnx("%s: %s", __func__, error->msg);
>> -			return;
>> -		}
>> -		gotweb_render_rss(c->tp);
>> -		return;
>> -	}
>> -
>> -	switch(qs->action) {
>> -	case BLAME:
>> -		error = got_get_repo_commits(c, 1);
>> -		if (error) {
>> -			log_warnx("%s: %s", __func__, error->msg);
>> -			goto err;
>> -		}
>> -		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>> -			return;
>> -		gotweb_render_page(c->tp, gotweb_render_blame);
>> -		return;
>> -	case BLOB:
>> -		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>> -			return;
>> -		gotweb_render_page(c->tp, gotweb_render_blob);
>> -		return;
>>  	case BRIEFS:
>>  		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
>>  			return;
>> @@ -337,6 +310,15 @@ gotweb_process_request(struct request *c)
>>  			return;
>>  		gotweb_render_page(c->tp, gotweb_render_index);
>>  		return;
>> +	case RSS:
>> +		error = got_get_repo_tags(c, D_MAXSLCOMMDISP);
>> +		if (error)
>> +			goto err;
>> +		if (gotweb_reply_file(c, rss_ctype, repo_dir->name, ".rss")
>> +		    == -1)
>> +			return;
>> +		gotweb_render_rss(c->tp);
>> +		return;
>>  	case SUMMARY:
>>  		error = got_ref_list(&c->t->refs, c->t->repo, "refs/heads",
>>  		    got_ref_cmp_by_name, NULL);
>
>


-- 
Tracey Emery
Sent from my phone.