On 6/12/21 9:15 AM, Eric Faurot wrote:
On Wed, Jun 09, 2021 at 05:41:36PM -0400, Aisha Tammy wrote:
Hi,
Here is the updated diff, which removes table_proc and adds table_procexec
as the default backend when no backend name matches.
Hi.
I'm not opposed to the idea, but I have a couple of comments:
First, if the two implementations are not going to coexists,
we can just replace table_proc.c.
Sounds good with me :D
Second, since the goal is to expose the protocol directly, instead of
relying on wrappers (through opensmtpd-extras api), we should take
time to refine it properly before, and avoid having to bump it every
other day. For example, I don't see the point of adding timestamps in
the request.
My WIP LDAP wrapper does not use the timestamps. I have not been privy
to the historical context of the development of this protocol, so I do
not know why it exists.
I am fine with removing it.
Do we want to be case-sensitive on the commands?
I reused the current table_service_name function present in table.c and
hence set it to all-caps. I think keeping it such is not an issue but I
don't care if we move to lowercase.
I would prefer being case sensitive, so as not to overly-complicate our
checks.
Do we need some kind of handshake?
I do not see a need for this between the table-open and the
table-process. The table-process may do handshakes (or whatever) in its
backend, which we should not be worrying about.
Also, there can be long-term plan to use
the same process for different tables, or even for other backends.
Finally the implementation could be factorized a bit, but that's a
detail at this time. I think the close operation (is it really useful anyway?)
should use fclose() instead of kill(), and maybe wait() too?
I agree, I am not sure if the table-close is very useful. I am also fine
with moving it to fclose, though I don't know how to manually close a
table >.<
BTW, can I remove the table_check function? I haven't seen a use for it
even after scrounging around a bit.
Unless any comments, I'll send the next patch (soonish) with fclose and
timestamps, table_check removed.
Cheers,
Aisha
Eric.
