Re: Direct SSL connection and ALPN loose ends

On Tue, Jul 23, 2024 at 08:32:29PM +0300, Heikki Linnakangas wrote:
> All these new tests are a great asset when refactoring this again.

Thanks for doing that. The coverage, especially with v2, is going to
be really useful.

> Yeah, I'm also not too excited about the additional code in the backend, but
> I'm also not excited about writing another test C module just for this. I'm
> inclined to commit this as it is, but we can certainly revisit this later,
> since it's just test code.

The point would be to rely on the existing injection_points module,
with a new callback in it. The callbacks could be on a file of their
own in the module, for clarity. What you have is OK for me anyway, it
is good to add more options to developers in this area and this gets
used in core. That's also enough to manipulate the stack in or even
out of core.

> Here's a new rebased version with some minor cleanup. Notably, I added docs
> for the new IS_INJECTION_POINT_ATTACHED() macro.

0001 looks OK.

+ push @events, "backenderror" if $line =~ /error triggered for
injection point backend-/;
+ push @events, "v2error" if $line =~ /protocol version 2 error
triggered/;

Perhaps append an "injection_" for these two keywords?

+#include "storage/proc.h"

This inclusion in injection_point.c should not be needed.

> sets the FrontendProtocol global variable, but I think it's more
> straightforward to have the test code

The last sentence in the commit message of 0002 seems to be
unfinished.

Could you run a perltidy on 005_negotiate_encryption.pl? There are a
bunch of new diffs in it.
--
Michael