supervision - discussion about system services, daemon supervision, init, runlevel management, and tools such as s6 and runit
 help / color / mirror / Atom feed
From: "Laurent Bercot" <ska-supervision@skarnet.org>
To: "George Matsumura" <gorg@gorgnet.net>, supervision@list.skarnet.org
Subject: Re: [PATCH] Resolve PROTOREMOTEHOST or PROTOLOCALHOST when only one is specified
Date: Tue, 30 Jul 2024 08:02:41 +0000	[thread overview]
Message-ID: <em73c8e7e3-fe68-4b10-95cd-818dfede4641@0948b344.com> (raw)
In-Reply-To: <20240730071112.30487-2-gorg@gorgnet.net>

>Previously, specification of PROTOLOCALHOST on the command line with the -l
>option would lead to PROTOREMOTEHOST not being resolved. This did not seem to
>be the intended behaviour, especially as the call to s6dns_resolven_parse_g
>works with any combination of the state of previous hostname definitions. This
>change alters the conditional to conduct the necessary resolutions in all
>cases. I apologize if I misinterpreted or was mistaken about anything.

  Your diagnosis is correct! Indeed, the call to s6dns_resolven_parse_g
must happen when at least one of (localname, remotelen) is 0, which
means they haven't yet been resolved another way.


>+    if (!localname || !remotelen && !s6dns_resolven_parse_g(blob + !!localname, !localname + !remotelen, &infinite))

  This, however, is incorrect: && has precedence over ||, so the call
would not happen when localname is 0. You want parentheses around
that ||. :)

  I've pushed a fix. Thanks for the report!

  s6-tcpserver-access.c is more spaghetti and difficult to read than
normally would be warranted for a program that should be simple on
paper, and I'm not surprised it's not exempt of bugs. Unfortunately,
it's in the hot path of new TCP connections from clients, and it
performs network resolutions such as DNS, so it needs to be optimized
for low latency prior to any other consideration. Thus, it's a maze
of tests and DNS queries are only performed when there's no other
option, and always in parallel when there are several of them. That's
why it looks like a mess with variables and conditions everywhere
which is a good breeding spot for the kind of bug you found.

--
  Laurent


      reply	other threads:[~2024-07-30  8:02 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-30  7:11 George Matsumura via supervision
2024-07-30  8:02 ` Laurent Bercot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=em73c8e7e3-fe68-4b10-95cd-818dfede4641@0948b344.com \
    --to=ska-supervision@skarnet.org \
    --cc=gorg@gorgnet.net \
    --cc=supervision@list.skarnet.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).