On Wed, Jan 11 2023, Matthew John Cheetham via GitGitGadget wrote:
> From: Matthew John Cheetham <mjcheetham@xxxxxxxxxxx>
> [...]
> +static void credential_write_strvec(FILE *fp, const char *key,
> + const struct strvec *vec)
> +{
> + int i = 0;
> + const char *full_key = xstrfmt("%s[]", key);
> + for (; i < vec->nr; i++) {
> + credential_write_item(fp, full_key, vec->v[i], 0);
Style: Don't mismatch types if there's no good reason. Use "size_t i" here, also let's do:
for (size_t i = 0; ....
I.e. no reason to declare it earlier.
> + }
> + free((void*)full_key);
Just don't add a "const" to that "full_key" and skip the cast with
free() here.
> +++ b/t/helper/test-credential-helper-replay.sh
I see to my surprise that we have one existing *.sh helper in that
directory, but in any case...
> @@ -0,0 +1,14 @@
> +cmd=$1
> +teefile=$cmd-actual.cred
> +catfile=$cmd-response.cred
> +rm -f $teefile
> +while read line;
> +do
> + if test -z "$line"; then
> + break;
> + fi
> + echo "$line" >> $teefile
> +done
It looks like you're re-inventing "sed" here, isn't this whole loop just
sed -n -e '/^$/q' -n 'p'
And then you can skip the "rm" before, as you could just clobber the
thing.
> +if test "$cmd" = "get"; then
> + cat $catfile
> +fi
> diff --git a/t/t5556-http-auth.sh b/t/t5556-http-auth.sh
> index 65105a5a6a9..a8dbee6ca40 100755
> --- a/t/t5556-http-auth.sh
> +++ b/t/t5556-http-auth.sh
> @@ -27,6 +27,8 @@ PID_FILE="$TRASH_DIRECTORY"/pid-file.pid
> SERVER_LOG="$TRASH_DIRECTORY"/OUT.server.log
>
> PATH="$GIT_BUILD_DIR/t/helper/:$PATH" && export PATH
> +CREDENTIAL_HELPER="$GIT_BUILD_DIR/t/helper/test-credential-helper-replay.sh" \
> + && export CREDENTIAL_HELPER
...(continued from above): Let's just use write_script() here or
whatever, i.e. no reason to make this a global script, it's just used in
this one test, so it can set it up.
>
> test_expect_success 'setup repos' '
> test_create_repo "$REPO_DIR" &&
> @@ -92,15 +94,279 @@ start_http_server () {
>
> per_test_cleanup () {
> stop_http_server &&
> - rm -f OUT.*
> + rm -f OUT.* &&
> + rm -f *.cred &&
> + rm -f auth.config
> }
>
> test_expect_success 'http auth anonymous no challenge' '
> test_when_finished "per_test_cleanup" &&
> - start_http_server &&
> +
> + cat >auth.config <<-EOF &&
> + [auth]
> + allowAnonymous = true
Mixed tab/space. Use "\t" not 4x " " (ditto below).
