From: Omar Polo <op@omarpolo.com>
Subject: Re: gotwebd diff is too slow
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Wed, 14 Jun 2023 12:46:26 +0200

On 2023/06/14 11:35:55 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> >  		| MAX_COMMITS_DISPLAY NUMBER {
> > -			if ($2 > 0)
> > -				new_srv->max_commits_display = $2;
> > +			if ($2 <= 1) {
> > +				yyerror("invalid max_commits_display %lld",
> > +				    $2);
> > +				YYERROR;
> > +			}
> 
> We should tell users what the valid range is.
> In the error message, in the man page, or both.

agreed, but we don't currently document the ranges (nor the default
values really) in the man page, so I'll do that in a future commit.

For the time being I've changed the error string to match what we'd
get with a 'standard' strtonum call.

diff /home/op/w/got
commit - 5144d22b0c3bcf6611cc36e93a3a859fcc521277
path + /home/op/w/got
blob - aba5a591662378d9b7363a6435d153408a11fb87
file + gotwebd/got_operations.c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -311,7 +311,7 @@ got_get_repo_commits(struct request *c, int limit)
 }
 
 const struct got_error *
-got_get_repo_commits(struct request *c, int limit)
+got_get_repo_commits(struct request *c, size_t limit)
 {
 	const struct got_error *error = NULL;
 	struct got_object_id *id = NULL;
@@ -328,7 +328,10 @@ got_get_repo_commits(struct request *c, int limit)
 	char *in_repo_path = NULL, *repo_path = NULL, *file_path = NULL;
 	int chk_next = 0;
 
-	if (limit != 1 || srv->max_commits_display == 1) {
+	if (limit == 0)
+		return got_error(GOT_ERR_RANGE);
+
+	if (limit > 1) {
 		/*
 		 * Traverse one commit more than requested to provide
 		 * the next button.
blob - fae082679ec712db36cb01d56e5edb5857535ab7
file + gotwebd/gotwebd.h
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -509,7 +509,7 @@ const struct got_error *got_get_repo_commits(struct re
 const struct got_error *got_get_repo_owner(char **, struct request *);
 const struct got_error *got_get_repo_age(time_t *, struct request *,
     const char *);
-const struct got_error *got_get_repo_commits(struct request *, int);
+const struct got_error *got_get_repo_commits(struct request *, size_t);
 const struct got_error *got_get_repo_tags(struct request *, int);
 const struct got_error *got_get_repo_heads(struct request *);
 const struct got_error *got_open_diff_for_output(FILE **, struct request *);
blob - bd616e146ccf4605ac53053dafa9b17cc39b8b86
file + gotwebd/parse.y
--- gotwebd/parse.y
+++ gotwebd/parse.y
@@ -386,8 +386,12 @@ serveropts1	: REPOS_PATH STRING {
 				new_srv->max_repos_display = $2;
 		}
 		| MAX_COMMITS_DISPLAY NUMBER {
-			if ($2 > 0)
-				new_srv->max_commits_display = $2;
+			if ($2 <= 1) {
+				yyerror("max_commits_display is too small:"
+				    " %lld", $2);
+				YYERROR;
+			}
+			new_srv->max_commits_display = $2;
 		}
 		;